On Nov 16, 2013, at 3:12 PM, Myklebust, Trond <Trond.Myklebust@xxxxxxxxxx> wrote: > > On Nov 16, 2013, at 14:15, Weston Andros Adamson <dros@xxxxxxxxxx> wrote: > >> I'm going to rethink this, as the server may return arbitrarily long bitmaps. >> >> I'll probably just always allocate an extra page... > > Perhaps we should do the same thing for NFSv4 that we do in nfs3_proc_getacl and just have xdr_partial_copy_from_skb allocate the pages for us? This sounds good - the nice part about this is that we’ll be able to cache the ACL the first time we ask for it instead of having the first getacl fail due to not having enough buffer space (this is currently just used to get the ACL length for the next call). So I have a patch almost ready, but I still have the same problem as before: what value should we use for the ‘len’ argument of xdr_inline_pages? nfs3_xdr_enc_getacl3args uses "NFSACL_MAXPAGES << PAGE_SHIFT”, which is fine because the inline pages only hold the ACL data, but for nfs4 we must have enough room for: - a bitmap4 type - the length of the ACL data - the ACL data The ACL length is known beforehand (we know the max is NFS4ACL_MAXPAGES << PAGE_SHIFT), but from the spec it looks like there can be 2^32-1 words in a bitmap4 since it’s defined as "typedef uint32_t bitmap4<>” (no maximum size specified in the <>). We certainly cannot allocate this many pages, so maybe can we agree on a reasonable number and add it as a #define? I still don’t understand why we need to support more than three bitmaps here, it sounds like supporting a buggy server and that’s usually a no-no. Also, nfs4xdr.h has #defines: #define nfs4_fattr_bitmap_maxsz 4 ... #define decode_getacl_maxsz (op_decode_hdr_maxsz + \ nfs4_fattr_bitmap_maxsz + 1) so that looks like we only expect 3 bitmap words... -dros -- To unsubscribe from this list: send the line "unsubscribe linux-nfs" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html