Prev: [PATCH 0/5] blkdev: discard optimizations v2 RESEND
Next: [PATCH 4/5] powernow-k8: Fix frequency reporting
From: Benny Halevy on 25 Mar 2010 10:40
On Mar. 25, 2010, 16:07 +0200, Benny Halevy <bhalevy(a)panasas.com> wrote:
> On Mar. 25, 2010, 16:06 +0200, Al Viro <viro(a)ZenIV.linux.org.uk> wrote:
>> On Thu, Mar 25, 2010 at 03:52:25PM +0200, Benny Halevy wrote:
>>> On Mar. 25, 2010, 15:37 +0200, Al Viro <viro(a)ZenIV.linux.org.uk> wrote:
>>>> On Thu, Mar 25, 2010 at 03:30:22PM +0200, Boaz Harrosh wrote:
>>>>>> Let's try this: before do_lookup() call there add
>>>>>> if (*want_dir)
>>>>>> nd->flags |= LOOKUP_DIRECTORY;
>>>>> Yes this fixes it!!
>>>>> 2.6.34-rc2 plus above, now works, horay. (diff attached)
>>>>>
>>>>>> and see how does it behave.
>>>>>>
>>>>>> However, even if it does help, it doesn't explain everything. Normal
>>>>>> open() on a directory without O_DIRECTORY if flags shouldn't fail with
>>>>>> -EISDIR. How did that manage to avoid it all along?
>>>> Does open() of directory _without_ O_DIRECTORY work in e.g. vanilla 2.6.33?
>>>> It certainly does for local filesystems and it does for NFSv3; does it work
>>>> for NFSv4?
>>> No, it doesn't.
>>>
>>> # mount localhost:/usr0/nfs4export /mnt/localhost; strace cat /mnt/localhost/server 2>&1 | grep 'open.*server'; umount /mnt/localhost
>>> open("/mnt/localhost/server", O_RDONLY) = 3
>>>
>>> # mount -t nfs4 localhost:/ /mnt/localhost; strace cat /mnt/localhost/server 2>&1 | grep 'open.*server'; umount /mnt/localhost
>>> open("/mnt/localhost/server", O_RDONLY) = -1 EISDIR (Is a directory)
>> Gets better - if you do ls -l /mnt/localhost/server and then repeat that
>> open(), it'll succeed.
>
> Correct :)

So, previously (pre 1f36f77), do_last called do_path_lookup with
lookup_flags(open_flag)|LOOKUP_OPEN and lookup_flags did
- if (f & O_DIRECTORY)
- retval |= LOOKUP_DIRECTORY;

and this was dropped in 1f36f77, right?

Benny

> _______________________________________________
> pNFS mailing list
> pNFS(a)linux-nfs.org
> http://linux-nfs.org/cgi-bin/mailman/listinfo/pnfs
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo(a)vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
From: Trond Myklebust on 25 Mar 2010 14:00
On Thu, 2010-03-25 at 19:28 +0200, Boaz Harrosh wrote:
> On 03/25/2010 05:25 PM, Al Viro wrote:

> > I'm going to send a fix for O_DIRECTORY case (restoring the behaviour
> > we had in 2.6.33) today, but NFS side of things also needs to be dealt
> > with.
>
> Thank you Al for fixing this, I hope some capable NFS person will take
> that issue to heart.

The NFSv4 fix is a trivial 1 liner. Pushed to bugfixes.

Now to understand that screwed up xdr decode...

Trond
------------------------------------------------------------------------------------------
NFSv4: Fall back to ordinary lookup if nfs4_atomic_open() returns EISDIR
From: Trond Myklebust <Trond.Myklebust(a)netapp.com>

Signed-off-by: Trond Myklebust <Trond.Myklebust(a)netapp.com>
---

fs/nfs/dir.c | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)


diff --git a/fs/nfs/dir.c b/fs/nfs/dir.c
index c6f2750..be46f26 100644
--- a/fs/nfs/dir.c
+++ b/fs/nfs/dir.c
@@ -1025,12 +1025,12 @@ static struct dentry *nfs_atomic_lookup(struct inode *dir, struct dentry *dentry
res = NULL;
goto out;
/* This turned out not to be a regular file */
+ case -EISDIR:
case -ENOTDIR:
goto no_open;
case -ELOOP:
if (!(nd->intent.open.flags & O_NOFOLLOW))
goto no_open;
- /* case -EISDIR: */
/* case -EINVAL: */
default:
goto out;


--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo(a)vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
From: Boaz Harrosh on 25 Mar 2010 14:10
On 03/25/2010 07:59 PM, Trond Myklebust wrote:
> On Thu, 2010-03-25 at 19:28 +0200, Boaz Harrosh wrote:
>> On 03/25/2010 05:25 PM, Al Viro wrote:
>
>>> I'm going to send a fix for O_DIRECTORY case (restoring the behaviour
>>> we had in 2.6.33) today, but NFS side of things also needs to be dealt
>>> with.
>>
>> Thank you Al for fixing this, I hope some capable NFS person will take
>> that issue to heart.
>
> The NFSv4 fix is a trivial 1 liner. Pushed to bugfixes.
>

Thanks

> Now to understand that screwed up xdr decode...
>

Tell me if you need that I run any tests. I still have that
failing setup if you need it.

> Trond

Boaz

> ------------------------------------------------------------------------------------------
> NFSv4: Fall back to ordinary lookup if nfs4_atomic_open() returns EISDIR
> From: Trond Myklebust <Trond.Myklebust(a)netapp.com>
>
> Signed-off-by: Trond Myklebust <Trond.Myklebust(a)netapp.com>
> ---
>
> fs/nfs/dir.c | 2 +-
> 1 files changed, 1 insertions(+), 1 deletions(-)
>
>
> diff --git a/fs/nfs/dir.c b/fs/nfs/dir.c
> index c6f2750..be46f26 100644
> --- a/fs/nfs/dir.c
> +++ b/fs/nfs/dir.c
> @@ -1025,12 +1025,12 @@ static struct dentry *nfs_atomic_lookup(struct inode *dir, struct dentry *dentry
> res = NULL;
> goto out;
> /* This turned out not to be a regular file */
> + case -EISDIR:
> case -ENOTDIR:
> goto no_open;
> case -ELOOP:
> if (!(nd->intent.open.flags & O_NOFOLLOW))
> goto no_open;
> - /* case -EISDIR: */
> /* case -EINVAL: */
> default:
> goto out;
>
>

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo(a)vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
From: Trond Myklebust on 25 Mar 2010 14:20
On Thu, 2010-03-25 at 20:06 +0200, Boaz Harrosh wrote:
> On 03/25/2010 07:59 PM, Trond Myklebust wrote:
> > Now to understand that screwed up xdr decode...
> >
>
> Tell me if you need that I run any tests. I still have that
> failing setup if you need it.

Do you have a wireshark (binary) dump that includes that particular
getattr call and reply? That would help.

Trond

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo(a)vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
From: Boaz Harrosh on 25 Mar 2010 14:40
On 03/25/2010 08:18 PM, Trond Myklebust wrote:
> On Thu, 2010-03-25 at 20:06 +0200, Boaz Harrosh wrote:
>> On 03/25/2010 07:59 PM, Trond Myklebust wrote:
>>> Now to understand that screwed up xdr decode...
>>>
>>
>> Tell me if you need that I run any tests. I still have that
>> failing setup if you need it.
>
> Do you have a wireshark (binary) dump that includes that particular
> getattr call and reply? That would help.
>

Wahoo sorry, it's already 20:31 out here It'll have to wait til Sunday
(You know that Hebrew weekend).

Tell me if you still need it sure, NP, I will make one first thing Sunday
morning.

> Trond
>

Boaz
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo(a)vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
First  |  Prev  | 
Pages: 1 2 3 4 5 6 7
Prev: [PATCH 0/5] blkdev: discard optimizations v2 RESEND
Next: [PATCH 4/5] powernow-k8: Fix frequency reporting