Re: seekdir/readdir patch .. Call for Review.

On 5/2/15 12:17 AM, Konstantin Belousov wrote:
> On Fri, May 01, 2015 at 03:04:51PM +0800, Julian Elischer wrote:
>> if you are interested in readdir(3), seekdir(3) and telldir(3) then
>> you should look at
>>     https://reviews.freebsd.org/D2410
>>
>> this patches around a problem in seekdir() that breaks Samba.
>> Seekdir(3) will not work as expected when files prior to the point of
>> interest in directory have been deleted since the directory was opened.
>>
>> Windows clients using Samba cause both these things to happen, causing
>> the next readdir(3) after the bad seekdir(3) to skip some entries and
>> return the wrong file.
>>
>> Samba only needs to step back a single directory entry in the case
>> where it reads an entry and then discovers it can't fit it into the
>> buffer it is sending to the windows client. It turns out we can
>> reliably cater to Samba's requirement because the "last returned
>> element" is always still in memory, so with a little care, we can
>> set our filepointer back to it safely. (once)
>>
>> seekdir and readdir (and telldir()) need a complete rewrite along with
>> getdirentries() but that is more than a small edit like this.
> Can you explain your expectations from the whole readdir() vs. parallel
> directory modifications interaction ?  From what I understood so far,
> there is unlocked modification of the container and parallel iterator
> over the same container.  IMO, in such situation, whatever tweaks you
> apply to the iterator, it is still cannot be made reliable.
>
> Before making single-purpose changes to the libc readdir and seekdir
> code, or to the kernel code, it would be useful to state exact behaviour
> of the dirent machinery we want to see. No, 'make samba works in my
> situation' does not sound good enough.
Well samba is a MAJOR application for FreeBSD. there are many people
who combine the two, (e.g. FreeNAS, which suffers from this problem)
so taking it from "The Samba community does not recommend
running on FreeBSD due to problems with seekdir" to "Samba works
correctly on FreeBSD" is important..   Samba's behaviour here is governed
by Windows expectiations.

I am specifically NOT changing kernel code.. though in discussion with
jhb and others, it becomes clear that this will have to happen some time
in the future.

There is no way to make seekdir 100% reliable without going to exposing
cookies through getdirentries(), and even then it relies on the file 
systems
doing the right thing with the cookies. Don't let Perfect be the enemy 
of 'better'.
I think hte two changes here make seekdir much more reliable. not only 
does it
make the 'back up one entry' case 100% reliable, but it also makes any 
backwards
seek within the current buffer also work correctly.

The expectation is that if you read an entry and then "immediately" set
the pointer back by one so that you can read it again, that hte next 
readdir()
will once again return the same entry regardless of whether any 
deletes have
happenned between the seekdir and the readdir()
In other words if we 'pre-read' an item to find its size, and then 
read it again..
we get the same item.

the change is small, makes it more reliable and helps a number of
FreeBSD users (e.g. FreeNAS).
I don't see why it would be objectionable to anyone. unless you are
relying an samba failing to delete random files when you try delete a 
subdirectory.




>
>