optimization : some buffers for small writes

17 views
Skip to first unread message

Emmanuel Anne

unread,
Feb 21, 2011, 12:33:00 AM2/21/11
to zfs-...@googlegroups.com
I had the idea while working on a dumb game which saves its data by doing something like :
file.WriteByte
file.WriteWord
...
This kills totally the performance of zfs-fuse because it has no buffers for that. This is the kind of optimization which should be done by fuse normally, not by us, but since they don't do it...
I first optimized the game to work around this, and since it produced a huge speed improvement, I had the idea to try it directly in zfs-fuse.
Some programs are still running at the same speed, but some programs run much faster, a noticeable difference is on the launch time for firefox when the ~/.mozilla directory is on a zfs-fuse directory.

The implementation is super basic : if a write has a size < 4096 bytes, it goes to a buffer dynamically allocated in ram, and then this buffer is used for any following write until either :
 - the file is closed
 - the buffer grows until its size is over 128k
 - the file is written at a different offset (not after the last write)
 - the file is read.
Something to notice : the buffers are never flushed on timeout, so if you are paranoid about power failures, don't use this patch, they are flushed only in 1 of these cases above.
I have tested the patch since yesterday, and since it seems to work well for me, I decided to commit it to the master branch of my repository. You are welcome to test it if you want.

--
zfs-fuse git repository : http://rainemu.swishparty.co.uk/cgi-bin/gitweb.cgi?p=zfs;a=summary

Seth Heeren

unread,
Feb 21, 2011, 3:36:48 AM2/21/11
to zfs-fuse
Cheers and congrats m8! That looks nice. It is indeed a bit unexpected
that there wouldn't be anything for this at the fuse level [1] but hey
- surprises are there for our enjoyment (?) !.

I'm so curious: did you run a bonnie++ yet? I expect mad improvements
in the byte-wise writes...


Seth


[1] mental note to self: go check what ntfs-3g do with that

Emmanuel Anne

unread,
Feb 21, 2011, 7:15:00 AM2/21/11
to zfs-...@googlegroups.com
No didn't run any benchmark, but the results would not make much sense anyway, with this writing anything small is like writing it to a ramdisk.
Notice that another obvious problem of the approach is that you need some free space : when the write is done and buffered, it returns a success to the application without checking for free space, but in usual conditions a zfs filesystem should never be almost full, so it should be a safe assumption.
And I am not 100% sure for all versions of firefox, I just realized it was slow with firefox 3.6, but I switched to firefox 4 while having the ~/.mozilla directory on a non zfs fs. It seems faster though !

2011/2/21 Seth Heeren <sghe...@hotmail.com>
--
To post to this group, send email to zfs-...@googlegroups.com
To visit our Web site, click on http://zfs-fuse.net/

Brian Neu

unread,
Feb 21, 2011, 8:54:47 AM2/21/11
to zfs-...@googlegroups.com

While I have been wrong in the past, I was under the impression that one of the primary purposes of using zfs is to secure the data at all costs.  Now speed is a huge detriment to the current usability of the zfs-fuse, but if we compromise on integrity for speed, is it really zfs anymore?  Power failures happen, as do hard crashes.  I can see this as a great mount option, but I don't know if I would want it as standard for my own build.  Just a thought.

Emmanuel Anne

unread,
Feb 21, 2011, 11:52:53 AM2/21/11
to zfs-...@googlegroups.com
I tried to reply on my phone, and the message got lost apparently, so here we go again :
It's easy to turn this into an option, but notice that you can't loose much anyway, at max 128k / opened file for writing, whcih can happen in normal conditions without this patch too. The filesystem can't be compromised because it happens at the file level only.
But it's probably a good idea to turn this into an option for the "super paranoid ones" anyway ! :)

2011/2/21 Brian Neu <brian...@gmail.com>

While I have been wrong in the past, I was under the impression that one of the primary purposes of using zfs is to secure the data at all costs.  Now speed is a huge detriment to the current usability of the zfs-fuse, but if we compromise on integrity for speed, is it really zfs anymore?  Power failures happen, as do hard crashes.  I can see this as a great mount option, but I don't know if I would want it as standard for my own build.  Just a thought.

--
To post to this group, send email to zfs-...@googlegroups.com
To visit our Web site, click on http://zfs-fuse.net/

Emmanuel Anne

unread,
Feb 21, 2011, 3:34:22 PM2/21/11
to zfs-...@googlegroups.com

Don't worry it's easy to turn this into an option. But anyway you would not loose much at max 128k worth of updates per opened file which can happen even without this. The fs can't be compromised it's at the file level only.

sgheeren

unread,
Feb 21, 2011, 6:02:15 PM2/21/11
to zfs-...@googlegroups.com
On 02/21/2011 09:34 PM, Emmanuel Anne wrote:
>
> Don't worry it's easy to turn this into an option. But anyway you
> would not loose much at max 128k worth of updates per opened file
> which can happen even without this. The fs can't be compromised it's
> at the file level only.
>
While I largely agree, let me point out that the crucial thing is that
writes are not buffered when syncing. So it is critical that the
relevant open() flags are taken into account, explicit synch calls still
work etc.
You don't want your database journal to be 'buffered' just because it
consists of small writes. Because, that would allow the database to get
corrupted - even though all that is at the file level.

Now get me right: I think your patch does all the above, so it should be
safe, but it is a valid worry, because things (not the fs, but still
important) can get corrupted if not.

Seth

Emmanuel Anne

unread,
Feb 21, 2011, 6:27:41 PM2/21/11
to zfs-...@googlegroups.com
Yes you are right for that I had missed it (even though databases usually use quite large records).
Anyway I have another unexpected problem : it doesn't work with git !

That's quite crazy and it's related to an incredible coincidence :
when you use git commit, git creates an index.lock file and writes blocks of data to it (unbuffered), then writes a block of 1516 bytes (this one is buffered), and then check the file size by calling fstat !

Well fstat in fuse is translated to _getattr, except that the very last parameter which should contain the info about the opened file is NULL, they say in the docs "for a later use maybe". Well if it had not been null it would be easy to fix, but here it becomes quite complicated !

And it's quite stupid, I don't see any reason why git checks the file size before closing it ? There are workarounds like keeping an array of opened inodes with the related info that getattr does not pass, but if you add threads to this it could very quickly become crazy. So for now I'll just think about it for a while.

This is the only problem I found so far, and I am quite surprised by the very big number of programs which used these buffers actually (just start an x session with your home on zfs and the buffers are already used very much !).

I'll find a fix for that, but too bad fuse didn't finish _getattr correctly. We could send a mail, but it would take time to be handled so we want a quicker fix than that. Later then...

2011/2/22 sgheeren <sghe...@hotmail.com>
--
To post to this group, send email to zfs-...@googlegroups.com
To visit our Web site, click on http://zfs-fuse.net/

sgheeren

unread,
Feb 21, 2011, 6:58:35 PM2/21/11
to zfs-...@googlegroups.com
On 02/22/2011 12:27 AM, Emmanuel Anne wrote:
>
> And it's quite stupid, I don't see any reason why git checks the file
> size before closing it ?
I don't suggest you post anything like this near git devs. Linus and his
friend tend to be quite strong opinioned about tech stuff like that. And
the sad truth is that they are usually horribly right. :)

> This is the only problem I found so far, and I am quite surprised by
> the very big number of programs which used these buffers actually
> (just start an x session with your home on zfs and the buffers are
> already used very much !).

I was still intending to compare with ntfs-3g, allthough with the
current thoughts/info I'm starting to expect that ntfs has no such
buffering optimization. Much could also be learned by running the posix
compliance test again with the optimization on, I suppose. But I have
never before ran that beast

Seth

Emmanuel Anne

unread,
Feb 21, 2011, 7:51:28 PM2/21/11
to zfs-...@googlegroups.com
2011/2/22 sgheeren <sghe...@hotmail.com>

On 02/22/2011 12:27 AM, Emmanuel Anne wrote:
>
> And it's quite stupid, I don't see any reason why git checks the file
> size before closing it ?
I don't suggest you post anything like this near git devs. Linus and his
friend tend to be quite strong opinioned about tech stuff like that. And
the sad truth is that they are usually horribly right. :)

Yes git is really good usually and there is probably a very good reason for that, but out of context it seems quite crazy !

> This is the only problem I found so far, and I am quite surprised by
> the very big number of programs which used these buffers actually
> (just start an x session with your home on zfs and the buffers are
> already used very much !).
I was still intending to compare with ntfs-3g, allthough with the
current thoughts/info I'm starting to expect that ntfs has no such
buffering optimization. Much could also be learned by running the posix
compliance test again with the optimization on, I suppose. But I have
never before ran that beast

For ntfs-3g, they can mount with the dev option to give directly the block device associated with a given fs. So small writes like that might be mmapped by fuse directly. We can't do that in zfs-fuse for 2 reasons :
1) there can be more than 1 device associated to a fs
and 2) with the crcs it's impossible to do a direct operation like that.
So there is probably nothing specific in their code for small operations.

For the compliance test, yes good idea. I'll try that tomorrow.
I have something which works for git now, I finally added a list to be able to retrieve the file_info_t element from an inode for _getattr. I didn't bother with threads because if a program accessed getattr on 1 thread while the other were doing operations on the same file, the results would be unpredictable, so it's probably no use to add locks here.

git commit works again here, but it's too late to commit, I'll wait until tomorrow, and run this posix compliance test suite again (you should try it one day, a nice piece of software, you learn quite a few things just by looking at how it works !).

I also added a command line parameter to disable it (--no-buffers), and something to detect when a file is opened with FSYNC flag (or OSYNC), and disable buffers in this case.

Seth

--
To post to this group, send email to zfs-...@googlegroups.com
To visit our Web site, click on http://zfs-fuse.net/

sgheeren

unread,
Feb 22, 2011, 2:34:39 AM2/22/11
to zfs-...@googlegroups.com
On 02/22/2011 01:51 AM, Emmanuel Anne wrote:
> For ntfs-3g, they can mount with the dev option to give directly the
> block device associated with a given fs. So small writes like that
> might be mmapped by fuse directly. We can't do that in zfs-fuse for 2
> reasons :
> 1) there can be more than 1 device associated to a fs
> and 2) with the crcs it's impossible to do a direct operation like that.
> So there is probably nothing specific in their code for small operations.
Thanks for that explanation. It makes sense now!

> I didn't bother with threads because if a program accessed getattr on
> 1 thread while the other were doing operations on the same file, the
> results would be unpredictable, so it's probably no use to add locks
> here.

Agreed. However, let's make sure that we must return either A (before)
or B (after), but not any random racy value :)

>
> git commit works again here, but it's too late to commit, I'll wait
> until tomorrow, and run this posix compliance test suite again (you
> should try it one day, a nice piece of software, you learn quite a few
> things just by looking at how it works !).

Ok, I'll start by remembering where to ofind i. On the ntfs-3g project
page, wasn't it?


>
> I also added a command line parameter to disable it (--no-buffers),
> and something to detect when a file is opened with FSYNC flag (or
> OSYNC), and disable buffers in this case.

That's brilliant!

Emmanuel Anne

unread,
Feb 22, 2011, 7:11:43 AM2/22/11
to zfs-...@googlegroups.com
http://www.tuxera.com/community/posix-test-suite/

The 2009 version was never finished, so the one to use is still the 2008 one.
2009 has some tests about xattrs.

I was right not to commit last night. After some thought I decided to change the way it works : at 1st I had just adapted the size returned by _getattr if it had some buffers, but it might be wrong if the process writes in the middle of the file instead of at the end.
So now if _getattr is called for a file which has a buffer, the buffer is flushed 1st.
It created a ton of problems with threads, I won't give the details of all I had but it was messy.
So finally I have a lock for this new struct too.
It's way too much for something which is supposed to be so simple, so if one day fuse passes the right parameter to _getattr it should be removed.
I'll try to send a mail to them later about that.

Now I have something quite big to commit in 1 time, sorry !
I ran the pjd tests and all tests succeeded. Could not reproduce any problem, I think it's good for a commit now...

2011/2/22 sgheeren <sghe...@hotmail.com>

--
To post to this group, send email to zfs-...@googlegroups.com
To visit our Web site, click on http://zfs-fuse.net/

sgheeren

unread,
Feb 22, 2011, 7:13:22 AM2/22/11
to zfs-...@googlegroups.com
On 02/22/2011 01:11 PM, Emmanuel Anne wrote:
http://www.tuxera.com/community/posix-test-suite/

The 2009 version was never finished, so the one to use is still the 2008 one.
2009 has some tests about xattrs.

I was right not to commit last night. After some thought I decided to change the way it works : at 1st I had just adapted the size returned by _getattr if it had some buffers, but it might be wrong if the process writes in the middle of the file instead of at the end.
So now if _getattr is called for a file which has a buffer, the buffer is flushed 1st.
It created a ton of problems with threads, I won't give the details of all I had but it was messy.
So finally I have a lock for this new struct too.
It's way too much for something which is supposed to be so simple, so if one day fuse passes the right parameter to _getattr it should be removed.
I'll try to send a mail to them later about that.

Now I have something quite big to commit in 1 time, sorry !
I ran the pjd tests and all tests succeeded. Could not reproduce any problem, I think it's good for a commit now...
Ok, let's see it :)

sgheeren

unread,
Feb 22, 2011, 11:17:01 AM2/22/11
to zfs-...@googlegroups.com
On 02/22/2011 01:13 PM, sgheeren wrote:

Now I have something quite big to commit in 1 time, sorry !
I ran the pjd tests and all tests succeeded. Could not reproduce any problem, I think it's good for a commit now...
Ok, let's see it :)

Ok, I've seen it (and used to procrastinate a bit more...)

I have done a bit of review, resulting in the following changes. You know how I like to microcommit. Think of it this way: if you weren't happy about the 'big-ball-of-mud' commit, this review can restore your conscience :)
0001-filtering-for-pid-_does_-hurt.patch
0002-unshuffle-zfs_operations.c-so-we-can-see-what-change.patch
0003-no-need-to-redefine-int_zfs_enter.patch
0004-sanity-check-to-avoid-leaving-the-lock-hanging.patch
0005-review-and-simplify-free_fi.patch
0006-good-custom-on-reallocations-is-to-grow-by-a-factor-.patch
0007-inverted-error-code-not-by-design.patch
0008-being-anal-about-potential-integer-overflows-and-sig.patch
0009-sanity-checks-on-storing-fi-records.patch
0010-avoid-unrelated-changes.patch
0011-fuse-and-zfs-inodes-were-mixed-up-misplaced-FUSE2ZFS.patch
0012-centralized-buffer-flushing-logic-and-make-errors-go.patch
In part these are just reformatting the patch so it was easier for me to see what did _not_ change. However these could be serious:
 
0001-filtering-for-pid-_does_-hurt.patch
I strongly believe that all processes should know about file changes (e.g. size) exactly like in the unbuffered world. This is as it has been. (Think of it this way, if git decides to use forks to check for the filestat changes, it should still work).
0007-inverted-error-code-not-by-design.patch
ok, mild, but could be confusing
0011-fuse-and-zfs-inodes-were-mixed-up-misplaced-FUSE2ZFS.patch
this is really the most interesting one
0012-centralized-buffer-flushing-logic-and-make-errors-go.patch
this should make Brian (and me) a lot happier. Let's not forget any errors, just because they were buffered. My policy has been to try to follow as many commands as possible (e.g. _do_ fsync the disks even though there was an error flushing the buffer) _but_ never swallow an error. Syslog is our friend, and where posisble, fuse will get the error condition as well, even though belated because of the buffering.

In cases where the 'later' operation (that triggered the flush) resulted in an error, the 'old' error might not be reported to fuse, which is why I also croak to syslog.

PS.: If a client follows a small write (<4096) by a very large one (say, 512Mb) then your allocation algorithm will resize to the exisiting buffer before flushing. The buffer is never downsized until the file is closed, so this could lead to problems. Unlikely though, so I left it for now.

My work is here:

    $ git clone -b buffering git://zfs-fuse.sehe.nl/git/sehe
   
This is on unstable. If you really want, you can rebase onto yours [1], but you are still encouraged to get the fixes from unstable (I painstakenly merged your branch a long time ago, so you should not have any problem doing the reverse anymore. I still hope you will start to see the value in having a common starting point for the work).

Cheers,
Seth

[1] git rebase -i sehe.nl/buffering~12 --onto rainemu/master

sgheeren

unread,
Feb 22, 2011, 11:18:24 AM2/22/11
to zfs-...@googlegroups.com
PS. disclaimer: this entire review was done in an editor. I haven't
actually ran any tests

Emmanuel Anne

unread,
Feb 22, 2011, 3:05:29 PM2/22/11
to zfs-...@googlegroups.com
Well I have run a lot, and I have some bad news for you, it's very unstable for now, you won't get any serious error though, it's more like cache pollution.
I just fixed one, the flush in getattr was not enough because getattr is by passed by zfsfuse_stat on a lot of functions, so I moved this to zfsfuse_stat.
Needed to change a lot of functions calls in the process.
Also fixed the part where push didn't correctly reset the cache after it was full (the next data block went in buffers < 4096 bytes or not).

For now I know there is at least one problem left :
when typing
git clone https://github.com/behlendorf/zfs.git
I get an error (and of course I should not get this error and it does not happen if buffers are disabled).
Well I'd like something easier to reproduce because this one is really hard !

Anyway except this it's geting better and still very fast, currently typing this mail with my home using buffers, so it works rather well usually !
So I'd advice not to use it except if you want to help to find the last bug (use the --no_buffers command line option if you want to avoid it).

Also I committed a decent way to log what happens in zfs_operations (with a DEBUG_LEVEL macro), so what it means is that it changes a lot of things, and it's going to be very hard to look at your patches now.
But I'll look again at the pid problem, there might be something there.

2011/2/22 sgheeren <sghe...@hotmail.com>
PS. disclaimer: this entire review was done in an editor. I haven't
actually ran any tests
--
To post to this group, send email to zfs-...@googlegroups.com
To visit our Web site, click on http://zfs-fuse.net/

sgheeren

unread,
Feb 22, 2011, 3:25:52 PM2/22/11
to zfs-...@googlegroups.com
On 02/22/2011 09:05 PM, Emmanuel Anne wrote:
> Also I committed a decent way to log what happens in zfs_operations
> (with a DEBUG_LEVEL macro), so what it means is that it changes a lot
> of things, and it's going to be very hard to look at your patches now.
Well, never mind then. I'll just throw them. It is the only way I can
even start to look at your patches: by cleaning up the patches into a
form that allows me to see _what changed_ and what did not. Something
else is just plain impossible to review.

> But I'll look again at the pid problem, there might be something there.

There isn't a big problem, except that it is arbitrary to up-and-decide
that other processes do not need up to date information. I don't know
what you decide that for, but it seems a bit unexpected to me.
The comment (paranoid?) seemed to suggest you were doing this as some
kind of security measure, which seemed a bit off-track to me.

Emmanuel Anne

unread,
Feb 22, 2011, 5:18:57 PM2/22/11
to zfs-...@googlegroups.com
Yes it was when I was having a lot of thread problems, I was getting paranoid very quickly and decided to refuse most of the requests this way. Bad idea, I just removed it in the last commit.

Ok this time everything works, at least I can't produce any error anymore for now (even the famous git clone works now !). For info it was the full buffer flush from push which was broken. To fix it I learned about the wonders of strace -f, and added a new debug level message to zfs_operations.c, and I found that basic_write returned error 22 (invalid argument) when there was a full buffer flush.

It was quite crazy, but I think it was worth it, it works much better this way.

Sorry for the big commits, small commits are better, but when you debug a lot of things this way it's hard to have small parts.

Anyway I'll keep using it daily and see if I can break it again, but this time it's going to be much harder. For info all I broke during my tests was the places.sqlite from firefox (the bookmarks !), luckily I had a backup so I didn't loose anything !

2011/2/22 sgheeren <sghe...@hotmail.com>

--
To post to this group, send email to zfs-...@googlegroups.com
To visit our Web site, click on http://zfs-fuse.net/

Emmanuel Anne

unread,
Feb 23, 2011, 8:19:40 PM2/23/11
to zfs-...@googlegroups.com
I had another crash a few hours ago, very hard to reproduce, but I finally got a core with debug symbols from it. Apparently it's a thread problem, a buffer was used but it had already been freed !
Quite surprising, but indeed, if a thread calls zfsfuse_stat on a inode while another one releases it, the buffer might be freed just before zfsfuse_stat uses it.
I hadn't seen this possibility at first.
That's why I hate all these threads, they make the simplest tasks become very complicated.

Hopefully this was the last crash.!

By the way, I discovered a new plugin for vim to handle git : fugitive.
Looks much better than my last plugin for git !

2011/2/22 Emmanuel Anne <emmanu...@gmail.com>

Emmanuel Anne

unread,
Feb 24, 2011, 10:48:49 AM2/24/11
to zfs-...@googlegroups.com
Ok, this one is the ultimate patch for the buffers, I promise ! ;-)

In fact, I finally found there is a specific fuse callback for this kind of function : flush !
With it, all the mess for _getattr becomes useless, so I removed it, along with all the fi struct and its content, the code is much shorter now.

So 1 more callback : flush.
The manual flush in read and fsync is still necessary.
The good way to test this was to use apt-src for example :
apt-src install util-linux
while 1 thread downloads, another checks the file (or so it seems).
Actually the release callback in fuse is not called when the files are closed, it's called when the file handles are all released, so it's not the function to use when there is something to sync. flush is the one.

I have commited in 2 commits this time, but if you look at the details, you'll see that some info->lock appeared and disappeared in them, it was a test which was not useful after all.

I return to using zfs-fuse with full optimizations, no debug symbols, this time I am sure it's the good one !
Sorry for the unstability and all the mails in 3 days !

2011/2/24 Emmanuel Anne <emmanu...@gmail.com>

Seth Heeren

unread,
Feb 25, 2011, 4:28:24 AM2/25/11
to zfs-fuse
Sounds like making sense.

Will have look later

On 24 feb, 16:48, Emmanuel Anne <emmanuel.a...@gmail.com> wrote:
> Ok, this one is the ultimate patch for the buffers, I promise ! ;-)
>
> In fact, I finally found there is a specific fuse callback for this kind of
> function : flush !
> With it, all the mess for _getattr becomes useless, so I removed it, along
> with all the fi struct and its content, the code is much shorter now.
>
> So 1 more callback : flush.
> The manual flush in read and fsync is still necessary.
> The good way to test this was to use apt-src for example :
> apt-src install util-linux
> while 1 thread downloads, another checks the file (or so it seems).
> Actually the release callback in fuse is not called when the files are
> closed, it's called when the file handles are all released, so it's not the
> function to use when there is something to sync. flush is the one.
>
> I have commited in 2 commits this time, but if you look at the details,
> you'll see that some info->lock appeared and disappeared in them, it was a
> test which was not useful after all.
>
> I return to using zfs-fuse with full optimizations, no debug symbols, this
> time I am sure it's the good one !
> Sorry for the unstability and all the mails in 3 days !
>
> 2011/2/24 Emmanuel Anne <emmanuel.a...@gmail.com>
>
>
>
>
>
> > I had another crash a few hours ago, very hard to reproduce, but I finally
> > got a core with debug symbols from it. Apparently it's a thread problem, a
> > buffer was used but it had already been freed !
> > Quite surprising, but indeed, if a thread calls zfsfuse_stat on a inode
> > while another one releases it, the buffer might be freed just before
> > zfsfuse_stat uses it.
> > I hadn't seen this possibility at first.
> > That's why I hate all these threads, they make the simplest tasks become
> > very complicated.
>
> > Hopefully this was the last crash.!
>
> > By the way, I discovered a new plugin for vim to handle git : fugitive.
> > Looks much better than my last plugin for git !
>
> > 2011/2/22 Emmanuel Anne <emmanuel.a...@gmail.com>
>
> > Yes it was when I was having a lot of thread problems, I was getting
> >> paranoid very quickly and decided to refuse most of the requests this way.
> >> Bad idea, I just removed it in the last commit.
>
> >> Ok this time everything works, at least I can't produce any error anymore
> >> for now (even the famous git clone works now !). For info it was the full
> >> buffer flush from push which was broken. To fix it I learned about the
> >> wonders of strace -f, and added a new debug level message to
> >> zfs_operations.c, and I found that basic_write returned error 22 (invalid
> >> argument) when there was a full buffer flush.
>
> >> It was quite crazy, but I think it was worth it, it works much better this
> >> way.
>
> >> Sorry for the big commits, small commits are better, but when you debug a
> >> lot of things this way it's hard to have small parts.
>
> >> Anyway I'll keep using it daily and see if I can break it again, but this
> >> time it's going to be much harder. For info all I broke during my tests was
> >> the places.sqlite from firefox (the bookmarks !), luckily I had a backup so
> >> I didn't loose anything !
>
> >> 2011/2/22 sgheeren <sghee...@hotmail.com>
>
> >>> On 02/22/2011 09:05 PM, Emmanuel Anne wrote:
> >>> > Also I committed a decent way to log what happens in zfs_operations
> >>> > (with a DEBUG_LEVEL macro), so what it means is that it changes a lot
> >>> > of things, and it's going to be very hard to look at your patches now.
> >>> Well, never mind then. I'll just throw them. It is the only way I can
> >>> even start to look at your patches: by cleaning up the patches into a
> >>> form that allows me to see _what changed_ and what did not. Something
> >>> else is just plain impossible to review.
>
> >>> > But I'll look again at the pid problem, there might be something there.
> >>> There isn't a big problem, except that it is arbitrary to up-and-decide
> >>> that other processes do not need up to date information. I don't know
> >>> what you decide that for, but it seems a bit unexpected to me.
> >>> The comment (paranoid?) seemed to suggest you were doing this as some
> >>> kind of security measure, which seemed a bit off-track to me.
>
> >>> --
> >>> To post to this group, send email to zfs-...@googlegroups.com
> >>> To visit our Web site, click onhttp://zfs-fuse.net/

Emmanuel Anne

unread,
Feb 25, 2011, 4:51:41 AM2/25/11
to zfs-...@googlegroups.com
Good you didn't take a look to soon, there was still a problem after all !

I was too optimistic yesterday, there is really a bug in fuse for handling getattr when we add a cache for write operations. I described it in the commit of today, but here it is again :
if a program uses stat to get the size of a file before the file is closed then fuse sends no flush command to the program (which is normal), and since the fi parameter passed to open is not passed to getattr, the data of the cache is lost here.
I should really send this mail after all. As usual I discovered this very late yesterday !

I have 3 tests now for this thing, and only the last one failed because of this : git commit -a.
So I was obliged to re-add something for getattr (not zfsfuse_stat this time, because this one is handled by flush now. It's only about getattr now), but I did something lighter this time. The list is updated and used as little as possible, and when used in getattr it doesn't flush the cache, it guesses what the size would be if the buffer was flushed instead and fixes the size returned by getattr with this.
Tested with : git commit -a (same which failed yesterday)
sudo apt-src install util-linux
git clone something
(all these tests use more than 1 thread. There is also a simple test I did with stat and fwrite for the problem of yesterday, this one works too now).

Everything works, and it should even be a little faster than the old get_info versions.

2011/2/25 Seth Heeren <sghe...@hotmail.com>
To visit our Web site, click on http://zfs-fuse.net/

sgheeren

unread,
Feb 25, 2011, 3:48:08 PM2/25/11
to zfs-...@googlegroups.com
On 02/24/2011 04:48 PM, Emmanuel Anne wrote:
>
> I return to using zfs-fuse with full optimizations, no debug symbols,
> this time I am sure it's the good one !
> Sorry for the unstability and all the mails in 3 days !
Don't apologize for experimenting/innovating :) I think it is good that
there is the clear 'official' repo though, so things can incubate before
anyone accidentally stumbles onto it.


Reply all
Reply to author
Forward
0 new messages