* Re: [PATCH 3/7] quota: add new quotactl Q_GETNEXTQUOTA
[not found] ` <1453435644-32261-4-git-send-email-sandeen@redhat.com>
@ 2016-01-22 9:28 ` Jan Kara
2016-01-22 13:58 ` Eric Sandeen
0 siblings, 1 reply; 2+ messages in thread
From: Jan Kara @ 2016-01-22 9:28 UTC (permalink / raw
To: Eric Sandeen; +Cc: linux-fsdevel, linux-api, jack, xfs
On Thu 21-01-16 22:07:20, Eric Sandeen wrote:
> Q_GETNEXTQUOTA is exactly like Q_GETQUOTA, except that it
> will return quota information for the id equal to or greater
> than the id requested. In other words, if the requested id has
> no quota, the command will return quota information for the
> next higher id which does have a quota set. If no higher id
> has an active quota, -ESRCH is returned.
>
> This allows filesystems to do efficient iteration in kernelspace,
> much like extN filesystems do in userspace when asked to report
> all active quotas.
>
> This does require a new data structure for userspace, as the
> current structure does not include an ID for the returned quota
> information.
>
> Today, Ext4 with a hidden quota inode requires getpwent-style
> iterations, and for systems which have i.e. LDAP backends,
> this can be very slow, or even impossible if iteration is not
> allowed in the configuration.
>
> Signed-off-by: Eric Sandeen <sandeen@redhat.com>
Same comments as for XFS version apply here. Furthermore:
> diff --git a/fs/quota/compat.c b/fs/quota/compat.c
> index fb1892f..80773a4 100644
> --- a/fs/quota/compat.c
> +++ b/fs/quota/compat.c
> @@ -19,6 +19,19 @@ struct compat_if_dqblk {
> compat_uint_t dqb_valid;
> };
>
> +struct compat_if_nextdqblk {
> + compat_u64 dqb_bhardlimit;
> + compat_u64 dqb_bsoftlimit;
> + compat_u64 dqb_curspace;
> + compat_u64 dqb_ihardlimit;
> + compat_u64 dqb_isoftlimit;
> + compat_u64 dqb_curinodes;
> + compat_u64 dqb_btime;
> + compat_u64 dqb_itime;
> + compat_uint_t dqb_valid;
> + compat_uint_t dqb_id;
> +};
> +
Is there a need for compat version of this structure? Everything is
naturally aligned and the size is a multiple of 8 bytes. But these things
keep surprising me... Added CC to linux-api in a hope that there's someone
who definitely knows.
On a side note when I have API people attention: AFAIU we need
struct compat_if_dqblk (defined in fs/quota/compat.c) because its size is
not multiple of 8 bytes, right? But what is then then reason for get_user()
/ put_user() when the structure is copied into userspace (see below)?
copy_in_user() should have copied everything...
if (copy_in_user(compat_dqblk, dqblk, sizeof(*compat_dqblk)) ||
get_user(data, &dqblk->dqb_valid) ||
put_user(data, &compat_dqblk->dqb_valid))
ret = -EFAULT;
Honza
--
Jan Kara <jack@suse.com>
SUSE Labs, CR
_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs
^ permalink raw reply [flat|nested] 2+ messages in thread
* Re: [PATCH 3/7] quota: add new quotactl Q_GETNEXTQUOTA
2016-01-22 9:28 ` [PATCH 3/7] quota: add new quotactl Q_GETNEXTQUOTA Jan Kara
@ 2016-01-22 13:58 ` Eric Sandeen
0 siblings, 0 replies; 2+ messages in thread
From: Eric Sandeen @ 2016-01-22 13:58 UTC (permalink / raw
To: Jan Kara, Eric Sandeen; +Cc: linux-fsdevel, linux-api, xfs
On 1/22/16 3:28 AM, Jan Kara wrote:
> On Thu 21-01-16 22:07:20, Eric Sandeen wrote:
...
> Same comments as for XFS version apply here.
*nod*
> Furthermore:
>
>> diff --git a/fs/quota/compat.c b/fs/quota/compat.c
>> index fb1892f..80773a4 100644
>> --- a/fs/quota/compat.c
>> +++ b/fs/quota/compat.c
>> @@ -19,6 +19,19 @@ struct compat_if_dqblk {
>> compat_uint_t dqb_valid;
>> };
>>
>> +struct compat_if_nextdqblk {
>> + compat_u64 dqb_bhardlimit;
>> + compat_u64 dqb_bsoftlimit;
>> + compat_u64 dqb_curspace;
>> + compat_u64 dqb_ihardlimit;
>> + compat_u64 dqb_isoftlimit;
>> + compat_u64 dqb_curinodes;
>> + compat_u64 dqb_btime;
>> + compat_u64 dqb_itime;
>> + compat_uint_t dqb_valid;
>> + compat_uint_t dqb_id;
>> +};
>> +
>
> Is there a need for compat version of this structure? Everything is
> naturally aligned and the size is a multiple of 8 bytes. But these things
> keep surprising me... Added CC to linux-api in a hope that there's someone
> who definitely knows.
Ok, yeah, I wasn't sure. I'll take all the compat stuff out of this one,
and can do a separate patch if needed, but I bet you're right, it's probably
not. I did set up to test 32-compat calls, so I'll just test w/o this.
I'd be perfectly happy to drop it. :)
Thanks for the review, Jan.
-Eric
^ permalink raw reply [flat|nested] 2+ messages in thread
end of thread, other threads:[~2016-01-22 13:58 UTC | newest]
Thread overview: 2+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
[not found] <1453435644-32261-1-git-send-email-sandeen@redhat.com>
[not found] ` <1453435644-32261-4-git-send-email-sandeen@redhat.com>
2016-01-22 9:28 ` [PATCH 3/7] quota: add new quotactl Q_GETNEXTQUOTA Jan Kara
2016-01-22 13:58 ` Eric Sandeen
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).