nfs accesses of FAT fs folders causes deadlock on lock_super

nfs accesses of FAT fs folders causes deadlock on lock_super

[Fredrick Prashanth John Berchmans]

Hi,

A FAT fs folder was shared using nfs. Accessing it over nfs causes
deadlock on lock_super since
it is acquired by both __fat_readdir and vfat_lookup in the following
call chain.

Call chain
#lock_super <----causes deadlock
#vfat_lookup
#__lookup_hash
#lookup_one_len
#compose_entry_fh
#encode_entry
#nfs3svc_encode_entry_plus
#__fat_readdir <---does lock_super
#fat_readdir
#vfs_readdir
#nfsd_readdir
#nfsd3_proc_readdirplus
#nfsd_dispatch
#svc_process
#nfsd


Is lock_super needed in the vfat_lookup?
Can it be removed?

The box was running kernel version 2.6.27.10.
But I think the FAT code has not changed much.

Thanks,
Fredrick

Re: nfs accesses of FAT fs folders causes deadlock on lock_super

[Christoph Hellwig]

On Mon, May 03, 2010 at 10:59:42PM -0700, Fredrick Prashanth John Berchmans wrote:
> The box was running kernel version 2.6.27.10.
> But I think the FAT code has not changed much.

But the nfsd code has and now doesn't call lookup from the filldir
callback anymore.

Re: nfs accesses of FAT fs folders causes deadlock on lock_super

[Fredrick Prashanth John Berchmans]

On Tue, May 4, 2010 at 3:32 AM, Christoph Hellwig <hch@infradead.org> wrote:
> On Mon, May 03, 2010 at 10:59:42PM -0700, Fredrick Prashanth John Berchmans wrote:
>> The box was running kernel version 2.6.27.10.
>> But I think the FAT code has not changed much.
>
> But the nfsd code has and now doesn't call lookup from the filldir
> callback anymore.
>
>

Ah..I just noticed the changes in the nfsd code.

But in 2.6.27.10, can it be fixed by removing the lock_super in the
vfat_lookup.?
Because I saw some discussions long back of regressions due to
replacing lock_kernel by lock_super in FAT code.
Or should the sb mutex be unlocked before calling the callback filldir
in the __fat_readdir?

Thanks,
Fredrick

Re: nfs accesses of FAT fs folders causes deadlock on lock_super

[OGAWA Hirofumi]

Fredrick Prashanth John Berchmans <fredrickprashanth@gmail.com> writes:

> On Tue, May 4, 2010 at 3:32 AM, Christoph Hellwig <hch@infradead.org> wrote:
>> On Mon, May 03, 2010 at 10:59:42PM -0700, Fredrick Prashanth John Berchmans wrote:
>>> The box was running kernel version 2.6.27.10.
>>> But I think the FAT code has not changed much.
>>
>> But the nfsd code has and now doesn't call lookup from the filldir
>> callback anymore.
>>
>>
>
> Ah..I just noticed the changes in the nfsd code.
>
> But in 2.6.27.10, can it be fixed by removing the lock_super in the
> vfat_lookup.?

Basically, yes.

> Because I saw some discussions long back of regressions due to
> replacing lock_kernel by lock_super in FAT code.

Yes. Current way is ultra safer side to convert BKL. Another side was
just to remove BKL. However, at least for now, we are using current way.

So, if current way had a problem, just remove it instead of trying to
fix (then, review).

Thanks.
-- 
OGAWA Hirofumi <hirofumi@mail.parknet.co.jp>

Re: nfs accesses of FAT fs folders causes deadlock on lock_super

[Fredrick Prashanth John Berchmans]

On Tue, May 4, 2010 at 3:32 AM, Christoph Hellwig <hch@infradead.org> wrote:
> On Mon, May 03, 2010 at 10:59:42PM -0700, Fredrick Prashanth John Berchmans wrote:
>> The box was running kernel version 2.6.27.10.
>> But I think the FAT code has not changed much.
>
> But the nfsd code has and now doesn't call lookup from the filldir
> callback anymore.
>
>

I see from nfsd code, it seems the compose_entry_fh still calls
lookup_one_len, which is now
called from encode_entryplus_baggage instead of entry_encode.
Am I right?

Re: nfs accesses of FAT fs folders causes deadlock on lock_super

[OGAWA Hirofumi]

Fredrick Prashanth John Berchmans <fredrickprashanth@gmail.com> writes:

> On Tue, May 4, 2010 at 3:32 AM, Christoph Hellwig <hch@infradead.org> wrote:
>> On Mon, May 03, 2010 at 10:59:42PM -0700, Fredrick Prashanth John Berchmans wrote:
>>> The box was running kernel version 2.6.27.10.
>>> But I think the FAT code has not changed much.
>>
>> But the nfsd code has and now doesn't call lookup from the filldir
>> callback anymore.
>>
>>
>
> I see from nfsd code, it seems the compose_entry_fh still calls
> lookup_one_len, which is now
> called from encode_entryplus_baggage instead of entry_encode.
> Am I right?

As far as I can see, nfsd is buffering the result of readdir, then do
jobs (encode_fh, etc.) with it on other context of readdir.

Aren't you missing a buffering of nfsd_buffered_readdir()?

Thanks.
-- 
OGAWA Hirofumi <hirofumi@mail.parknet.co.jp>

Re: nfs accesses of FAT fs folders causes deadlock on lock_super

[Fredrick Prashanth John Berchmans]

> As far as I can see, nfsd is buffering the result of readdir, then do
> jobs (encode_fh, etc.) with it on other context of readdir.
>
> Aren't you missing a buffering of nfsd_buffered_readdir()?

Sorry I missed that.
My boxes run 2.6.27 kernel which does not have nfsd_buffered_readdir.

Thanks,
Fredrick