Commit 303a7805 authored by Anna Schumaker's avatar Anna Schumaker
Browse files

NFSv4.2: Rework scratch handling for READ_PLUS (again) 



I found that the read code might send multiple requests using the same
nfs_pgio_header, but nfs4_proc_read_setup() is only called once. This is
how we ended up occasionally double-freeing the scratch buffer, but also
means we set a NULL pointer but non-zero length to the xdr scratch
buffer. This results in an oops the first time decoding needs to copy
something to scratch, which frequently happens when decoding READ_PLUS
hole segments.

I fix this by moving scratch handling into the pageio read code. I
provide a function to allocate scratch space for decoding read replies,
and free the scratch buffer when the nfs_pgio_header is freed.

Fixes: fbd2a05f (NFSv4.2: Rework scratch handling for READ_PLUS)
Signed-off-by: default avatarAnna Schumaker <Anna.Schumaker@Netapp.com>
parent 8d18f6c5

fs/nfs/internal.h

+1 −0
Original line number Diff line number Diff line
@@ -493,6 +493,7 @@ extern const struct nfs_pgio_completion_ops nfs_async_read_completion_ops; 
extern void nfs_pageio_init_read(struct nfs_pageio_descriptor *pgio,

			struct inode *inode, bool force_mds,

			const struct nfs_pgio_completion_ops *compl_ops);

extern bool nfs_read_alloc_scratch(struct nfs_pgio_header *hdr, size_t size);

extern int nfs_read_add_folio(struct nfs_pageio_descriptor *pgio,

			       struct nfs_open_context *ctx,

			       struct folio *folio);

fs/nfs/nfs42.h

+1 −0
Original line number Diff line number Diff line
@@ -13,6 +13,7 @@ 
 * more? Need to consider not to pre-alloc too much for a compound.

 */

#define PNFS_LAYOUTSTATS_MAXDEV (4)

#define READ_PLUS_SCRATCH_SIZE (16)



/* nfs4.2proc.c */

#ifdef CONFIG_NFS_V4_2

fs/nfs/nfs42xdr.c

+1 −1
Original line number Diff line number Diff line
@@ -1433,7 +1433,7 @@ static int nfs4_xdr_dec_read_plus(struct rpc_rqst *rqstp, 
	struct compound_hdr hdr;

	int status;



	xdr_set_scratch_buffer(xdr, res->scratch, sizeof(res->scratch));

	xdr_set_scratch_buffer(xdr, res->scratch, READ_PLUS_SCRATCH_SIZE);



	status = decode_compound_hdr(xdr, &hdr);

	if (status)

fs/nfs/nfs4proc.c

+1 −12
Original line number Diff line number Diff line
@@ -5438,18 +5438,8 @@ static bool nfs4_read_plus_not_supported(struct rpc_task *task, 
	return false;

}



static inline void nfs4_read_plus_scratch_free(struct nfs_pgio_header *hdr)

{

	if (hdr->res.scratch) {

		kfree(hdr->res.scratch);

		hdr->res.scratch = NULL;

	}

}



static int nfs4_read_done(struct rpc_task *task, struct nfs_pgio_header *hdr)

{

	nfs4_read_plus_scratch_free(hdr);



	if (!nfs4_sequence_done(task, &hdr->res.seq_res))

		return -EAGAIN;

	if (nfs4_read_stateid_changed(task, &hdr->args))
@@ -5469,8 +5459,7 @@ static bool nfs42_read_plus_support(struct nfs_pgio_header *hdr, 
	/* Note: We don't use READ_PLUS with pNFS yet */

	if (nfs_server_capable(hdr->inode, NFS_CAP_READ_PLUS) && !hdr->ds_clp) {

		msg->rpc_proc = &nfs4_procedures[NFSPROC4_CLNT_READ_PLUS];

		hdr->res.scratch = kmalloc(32, GFP_KERNEL);

		return hdr->res.scratch != NULL;

		return nfs_read_alloc_scratch(hdr, READ_PLUS_SCRATCH_SIZE);

	}

	return false;

}

fs/nfs/read.c

+10 −0
Original line number Diff line number Diff line
@@ -47,6 +47,8 @@ static struct nfs_pgio_header *nfs_readhdr_alloc(void) 


static void nfs_readhdr_free(struct nfs_pgio_header *rhdr)

{

	if (rhdr->res.scratch != NULL)

		kfree(rhdr->res.scratch);

	kmem_cache_free(nfs_rdata_cachep, rhdr);

}
@@ -108,6 +110,14 @@ void nfs_pageio_reset_read_mds(struct nfs_pageio_descriptor *pgio) 
}

EXPORT_SYMBOL_GPL(nfs_pageio_reset_read_mds);



bool nfs_read_alloc_scratch(struct nfs_pgio_header *hdr, size_t size)

{

	WARN_ON(hdr->res.scratch != NULL);

	hdr->res.scratch = kmalloc(size, GFP_KERNEL);

	return hdr->res.scratch != NULL;

}

EXPORT_SYMBOL_GPL(nfs_read_alloc_scratch);



static void nfs_readpage_release(struct nfs_page *req, int error)

{

	struct folio *folio = nfs_page_to_folio(req);