linux-ext4 - Re: [PATCH v13 45/51] sunrpc: Allow to demand-allocate pages to encode into

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:	Thu, 5 Nov 2015 12:07:05 +0100
From:	Andreas Gruenbacher <agruenba@...hat.com>
To:	Trond Myklebust <trond.myklebust@...marydata.com>
Cc:	Alexander Viro <viro@...iv.linux.org.uk>,
	"Theodore Ts'o" <tytso@....edu>,
	Andreas Dilger <adilger.kernel@...ger.ca>,
	"J. Bruce Fields" <bfields@...ldses.org>,
	Jeff Layton <jlayton@...chiereds.net>,
	Anna Schumaker <anna.schumaker@...app.com>,
	Dave Chinner <david@...morbit.com>,
	linux-ext4 <linux-ext4@...r.kernel.org>,
	XFS Developers <xfs@....sgi.com>,
	Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
	Linux FS-devel Mailing List <linux-fsdevel@...r.kernel.org>,
	Linux NFS Mailing List <linux-nfs@...r.kernel.org>,
	linux-cifs@...r.kernel.org,
	Linux API Mailing List <linux-api@...r.kernel.org>
Subject: Re: [PATCH v13 45/51] sunrpc: Allow to demand-allocate pages to
 encode into

Trond,

On Tue, Nov 3, 2015 at 5:25 PM, Trond Myklebust
<trond.myklebust@...marydata.com> wrote:
> On Tue, Nov 3, 2015 at 10:17 AM, Andreas Gruenbacher
> <agruenba@...hat.com> wrote:
>> When encoding large, variable-length objects such as acls into xdr_bufs,
>> it is easier to allocate buffer pages on demand rather than precomputing
>> the required buffer size.
>
> NACK. We're not doing allocations from inside the XDR encoders. This
> can and should be done before calling into the SUNRPC layer.

an XDR-encoded ACL can be up to 64k (16 pages) in size. In practice,
large ACLs like that will almost never occur and almost all ACLs will
fit into a single page though.

The XDR-encoded ACL contains strings for the user and group names
which need to be looked up when the idmapper is used. Those lookups
are somewhat expensive; in addition, the lookup results can change
over time. When precomputing the size, allocating space, and then
encoding the ACL, we could run out of space when encoding.

So we could always allocate the maximum 16 pages, encode the acl, and
free the unused pages. This would be rather wasteful though.

Given how simple it is to allocate pages as we go, this seems the
better choice here. This doesn't break any existing code either; NULL
page pointers would have oopsed in xdr_get_next_encode_buffer before.

>From the memory management point of view, there is no difference in
preallocating GFP_NOFS pages and allocating them on demand; the pages
are allocated in the same task and locking context in both cases.

So could you please explain why you object to this change?

Thanks,
Andreas
--
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