Prev: [GIT PULL] AMD64 EDAC fixlet for 2.6.35-rc4
Next: iwl3945: HARDWARE GONE??
From: David Howells on 7 Jul 2010 11:30
Andreas Dilger <adilger(a)dilger.ca> wrote:

> I saw your patch to that effect. I'm of mixed feelings about this, since
> the S_* flags have traditionally been changed on an ad-hoc basis and I don't
> necessarily want to let this leak into the on-disk format of these flags for
> ext*.
>
> One way to ensure that this holds true is to have a compile-time assertion
> that the respective S_* flags match FS_*_FL and EXT_*_FL.

Something like this:

#if S_SYNC != FS_SYNC_FL || \
S_IMMUTABLE != FS_IMMUTABLE_FL || \
S_APPEND != FS_APPEND_FL || \
S_NOATIME != FS_NOATIME_FL || \
S_DIRSYNC != FS_DIRSYNC_FL
#error Ext2/3/4 assumes these equivalences
#endif

would do the trick.

> > (5) How often are these flags required? E.g. does it make more sense to
> > keep them as an additional result, or does it make sense to stick them
> > in the kstat and xstat structs, knowing that these are allocated on the
> > kernel stack maybe as three times if an ecryptfs file?
>
> If they aren't requested by userspace, the cost is mostly irrelevant. I
> think on OSX these flags are returned for every "ls" call, to mark the
> inodes with xattrs every time.

I suppose. I was thinking that they may have to be retrieved from the server.

More concerning to me is the effect of adding more fields to the kstat struct.

Nonetheless, having these flags around may be useful to CIFS, Samba, NFS and
NFSD as various of them may appear in those protocols. Certainly, SMB passes
a bit indicating compression around (ATTR_COMPRESSED).

David
--
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: David Howells on 9 Jul 2010 10:00
Michael Kerrisk <mtk.manpages(a)gmail.com> wrote:

> > � � � �struct xstat_parameters {
> > � � � � � � � �unsigned long long � � �request_mask;
>
> Poor name, since it's a value-result arg? Better maybe something like
> "field_mask"?

No. The contents of xstat_parameters aren't changed. request_mask is what
you're asking for, result_mask in the xstat struct is what you actually got.

result_mask may be more or less than request_mask as the filesystem isn't
obliged to supply anything you didn't ask for, and may not be able to supply
something you did ask for, and may give you stuff anyway that you didn't ask
for if it's trivial to do so.

> There is no XSTAT_REQUEST__GET_ANYWAY, AFAICS. I guess here you meant
> XSTAT_REQUEST__EXTENDED_STATS? Or?

Yep. I forgot to change that in the patch description.

> This case is almost certainly a user error, so why not simply return
> an error (-1 and ERANGE or E2BIG)? The above approach invites
> userspace errors of the form:
>
> if (xtat(...) < 0) { /* How users often check for error */
> /* I'll handle the error */
> } else {
> /* The call succeeded; I'm fine */
> }

I suppose.

> If you are looking for a way to inform the user about the required
> buffer size, I think it would be better to take a leaf from the
> getxattr(2) book: if 'buflen' is zero, then do nothing with the output
> arg, but return the size that would be required.

That's reasonable.

David
--
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
Prev: [GIT PULL] AMD64 EDAC fixlet for 2.6.35-rc4
Next: iwl3945: HARDWARE GONE??