Re: [RFC] quota: 64-bit limits with vfs

[Andreas Dilger <adilger@sun.com>]

On Mar 15, 2008  16:24 +0300, Andrew Perepechko wrote:
> On Saturday 15 March 2008 07:23:16 Andreas Dilger wrote:
> > In a few places you add new on-stack variables like "sb", but they aren't
> > used more than 1 or 2 times.  While it makes the code a tiny bit clearer
> > (though not largely so for "sb" because it is only dquot->dq_sb) it does
> > increase the stack usage, and that is never a good idea.
> 
> I agree. Probably, it makes sense to rollback sb change. Though this may lead
> to additional line breaks.

Line breaks don't consume stack space ;-).

> > Hmm, allocating "fakedquot" on the stack just to compare it to zero
> > doesn't seem like a good use of space.  What about doing the memcmp()
> > against page_address(ZERO_PAGE(0))?  It might be nice to have a permanent
> > mapping of ZERO_PAGE(0) like void *zero_buffer that can be used for this.
> 
> What do you think about something like this?

> 
> +static union v2_disk_dqblk emptydquot;
> +static union v2_disk_dqblk fakedquot[2] = {
> +	{.disk_dqblk_r0 = {.dqb_itime = __constant_cpu_to_le64(1LLU)}},
> +	{.disk_dqblk_r1 = {.dqb_itime = __constant_cpu_to_le64(1LLU)}}
> +};

Yes, since these structures are constant there are no multi-thread issues.
Not only does this use less stack space, but it is better performing as
well due to less memset() overhead.

NB - it seems you have whitespace at the end of some lines, please remove it.

>@@ -724,9 +724,7 @@ static int v2_read_dquot(struct dquot *d
>		else {
>			ret = 0;
>			/* We need to escape back all-zero structure */
>-			memset(&empty, 0, dqblksz);
>-			DQF_PUT(&empty, rev, dqb_itime, 1);
>-			if (!memcmp(&empty, &ddquot, dqblksz))
>+			if (!memcmp(&fakedquot[rev], &ddquot, dqblksz))
>				DQF_PUT(&ddquot, rev, dqb_itime, 0);
>		}

I wonder if there is a CPU instruction "compare memory with zero" that
would be available for something like this, that could be used like:

			if (!memzcmp(&ddquot, dqblksz))
				DQF_PUT(&ddquot, rev, dqb_itime, 0);

I know I've had to use a few hacks like this to check if some buffer is
zero filled, so having it efficiently done by the CPU would be a win.
It would be possible to use something like generic_find_first_bit():

#define memzcmp(ptr, bytes) (generic_find_first_bit(ptr, bytes*8) >= bytes*8)

which breaks down to an assembly instruction "bsfq" on x86_64 if the
parameter is constant.  That might make it more efficient to have 2
separate codepaths here with a constant "bytes" parameter so that the
larger memzcmp() can be optimized.

Cheers, Andreas
--
Andreas Dilger
Sr. Staff Engineer, Lustre Group
Sun Microsystems of Canada, Inc.