[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <4F96FD45.9080902@redhat.com>
Date: Tue, 24 Apr 2012 14:21:41 -0500
From: Eric Sandeen <sandeen@...hat.com>
To: Bernd Schubert <bernd.schubert@...m.fraunhofer.de>
CC: Andreas Dilger <adilger@...mcloud.com>, linux-ext4@...r.kernel.org,
Fan Yong <yong.fan@...mcloud.com>, bfields@...hat.com
Subject: Re: [PATCH 5 2/4] Return 32/64-bit dir name hash according to usage
type
On 4/24/12 11:10 AM, Bernd Schubert wrote:
> On 04/24/2012 12:42 AM, Andreas Dilger wrote:
>> On 2012-04-23, at 5:23 PM, Eric Sandeen wrote:
>>> I'm curious about the above as well as:
>>>
>>> case SEEK_END:
>>> if (unlikely(offset> 0))
>>> goto out_err; /* not supported for directories */
>>>
>>> The previous .llseek handler, and the generic handler for other
>>> filesystems, allow seeking past the end of the dir AFAICT. (not
>>> sure why you'd want to, but I don't see that you'd get an error
>>> back).
>>>
>>> Is there a reason to uniquely exclude it in ext4? Does that line up with POSIX?
>>
>> I don't know what the origin of this was... I don't think there is
>> a real reason for it except that it doesn't make any sense to do
>> so.
>>
>
> I think I added that. According to pubs.opengroup.org:
> (http://pubs.opengroup.org/onlinepubs/009695399/functions/seekdir.html)
>
> void seekdir(DIR *dirp, long loc);
>
> <quote>
>
> If the value of loc was not obtained from an earlier call to
> telldir(), or if a call to rewinddir() occurred between the call to
> telldir() and the call to seekdir(), the results of subsequent calls
> to readdir() are unspecified.
>
> </quote>
>
>
> As telldir(), which should correlate to 'case SEEK_CUR' will not
> provide invalid values, the behaviour is undefined.
>
>
> Also,
>
>
> case SEEK_END:
> [...]
> if (dx_dir)
> offset += ext4_get_htree_eof(file);
> else
> offset += inode->i_size;
> [...]
>
>
> if (!dx_dir) {
> if (offset > inode->i_sb->s_maxbytes)
> goto out_err;
> } else if (offset > ext4_get_htree_eof(file))
> goto out_err;
>
>
>
>
> Hence, the additional:
>
> case SEEK_END:
> if (unlikely(offset> 0))
> goto out_err; /* not supported for directories */
>
>
> is just a shortcut to avoid useless calculations.
>
> Unless I missed something, it only remains the question if could
> break existing applications relying on undefined behaviour. However,
> I have no idea how an application might trigger that?
(other lists removed at this point, this is ext4-specific)
I know I'm being a little pedantic w/ the late review here....
It seems like the only differences between ext4_dir_llseek and the old ext4_llseek are these:
1) For SEEK_END, we now return -EINVAL for a positive offset (i.e. past EOF)
2) For SEEK_END, we seek to ext4_get_htree_eof() not to inode->i_size
3) For SEEK_SET, we impose different limits for max offset
- s_maxbytes / ext4_get_htree_eof for !dx/dx, vs. s_bitmap_maxbytes/s_maxbytes
Do any of these changes relate to the hash collision problem? Are any of them uniquely
required for ext4, enough to warrant cut & paste of the vfs llseek code (again?)
What I'm getting at is: what are the reasons that we cannot use generic_file_llseek_size(),
maybe with a new argument to specify a non-standard location for SEEK_END. Such
a change would require a solid explanation, but it'd probably go in if it meant
one less seek implementation to worry about.
-Eric
--
To unsubscribe from this list: send the line "unsubscribe linux-ext4" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Powered by blists - more mailing lists