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

[Günter Kukkukk]

Am Mittwoch, 15. April 2009 schrieb Shirish Pargaonkar:
> 2009/3/20 Günter Kukkukk <linux at kukkukk.com>:
> > 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)
> >
> 
> Even forcing  a call to invalidate_remote_inode() does not seem to
> help if the server (f)opened file
> in with "a" mode instead of "w" mode i.e. if wrt.c calls fp = fopen
> (filename, "a"); instead of fp = fopen (filename, "w");

Here i see no difference when using fp = fopen (filename, "a") instead
of fp = fopen (filename, "w").
I works in both cases when I (temporarily) change the source to:

	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);
-			}
+			invalidate_remote_inode(direntry->d_inode);   /* !!!!! temp. test !!!!! /
		}
	}

I'm using Steve's latest git tree from kernel 2.6.30-rc2 and samba-3.3.x.


> 
> > 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
> >
>