[linux-cifs-client] tail -f /mounted/share/filename is not working

[Wilhelm Meier]

Am Mittwoch 08 April 2009 schrieb Shirish Pargaonkar:
> On Fri, Mar 20, 2009 at 10:50 PM, Günter Kukkukk <linux at kukkukk.com> 
wrote:
> > Am Freitag, 20. März 2009 schrieb Suresh Jayaraman:
> >> � wrote:
> >> > Hi Steve, Jeff,
> >> >
> >> > this one is on my todo list for more than a year now.
> >>
> >> May be this behavior has changed now?  I tried to reproduce this
> >> on 2.6.27.19 with directio option against a Samba-3.0.26 server.
> >> I could not reproduce this at all (I used tail instead of
> >> statgk). I'm seeing the stuff written with negligible delay on
> >> the client.
> >>
> >> Are you able to reproduce this with the recent kernels? If yes,
> >> are you getting more clues as to what is happening with a
> >> tcpdump/cifsFYI ?
> >
> > About a year ago i had some short irc talk with Jeff about this
> > tail -f .. problem, but we dropped it, cause other stuff had
> > higher priority.
> >
> > I actually do my tests on Steve's recent git kernel tree - so
> > 2.6.29-rc7 - against recent samba_v-3-3-test.
> > As i already mentioned
> >    "Btw - when using the mount option "directio", all _seems_ to
> > be fine." the tail -f ... problem is only seen here when
> > "directio" is _not_ used.
> >
> > The wire traffic to the server is only a repeated
> >   QUERY_PATH_INFO (Query File Unix Basic)
> > as requested periodically from the clients program fstat()
> >
> > Btw - my statgk applet has some advantages over 'tail' when
> > debugging this, by also listing the size (..) change - and
> > showing the ability to really read the appended line(s) - and
> > list it as ASCII and also as some first hex bytes (which are
> > often nil here).
> >
> > In inode.c --> cifs_revalidate() the var "invalidate_inode" is
> > correctly set true.
> >
> > To me, the problematic part is inside this code fragment:
> >
> > -----------------------
> >        if (invalidate_inode) {
> >        /* shrink_dcache not necessary now that cifs dentry ops
> >        are exported for negative dentries */
> > /*              if (S_ISDIR(direntry->d_inode->i_mode))
> >                        shrink_dcache_parent(direntry); */
> >                if (S_ISREG(direntry->d_inode->i_mode)) {
> >                        if (direntry->d_inode->i_mapping) {
> >                                wbrc =
> > filemap_fdatawait(direntry->d_inode->i_mapping); if (wbrc)
> >                                       
> > CIFS_I(direntry->d_inode)->write_behind_rc = wbrc; }
> >                        /* may eventually have to do this for open
> > files too */ if (list_empty(&(cifsInode->openFileList))) { /*
> > changed on server - flush read ahead pages */ cFYI(1,
> > ("Invalidating read ahead data on " "closed file"));
> > invalidate_remote_inode(direntry->d_inode); }
> >                }
> >        }
> >
> > -------------------------------
> >
> > The code path to use
> >     invalidate_remote_inode(direntry->d_inode);
> > is not hit.
> > (when i force "invalidate_remote_inode(direntry->d_inode);" here,
> > all looks much better)
> >
> > Sorry, when i'm wrong here. :-)
> >
> > Cheers, Günter
> >
> >> > How to test:
> >> >   server side: use "./wrt filename" or "./wrt -s 100 filename"
> >> >                to start writing/appending to a file on a
> >> > (samba) exported share (regarding the '-s' option, I though
> >> > writing large data would/could trigger some cifs "re-read
> >> > server" internals (page dirty...)) cifs client side: (one
> >> > might use 'tail -f /mnt/xyz/filename' as well) use ./statgk
> >> > /mnt/xyz/filename
> >> >
> >> > On the cifs client side "statgk" uses fstat() to get possible
> >> > _updated_ file info from the server (filesize, times, ...) -
> >> > cifs *realizes* (!) the size (times) change, but no "read
> >> > additional data" from the server is done.
> >>
> >> Thanks,
> >
> > _______________________________________________
> > linux-cifs-client mailing list
> > linux-cifs-client at lists.samba.org
> > https://lists.samba.org/mailman/listinfo/linux-cifs-client
>
> I think we should take out the check for closed files for that
> inode if (list_empty(&(cifsInode->openFileList))) {
> and invalidate pages of the inode instead of gating on whether the
> file is closed or not, if invalidate_inode gets marked for whatever
> reason (e.g. change in file size).

Do you experience this behaviour also with expanding / reading the 
file on the client side? 
The code-excerpt above doesn't show this case, but we have serious 
problems using kmail on cifs and the problems looks the same: kmail 
crashes with incorrect index-files, using directio corrects this! 

-- 
Wilhelm