On 2010-05-17 20:56, Alexandros Batsakis wrote:
> CB_RECALL_ANY serves as a hint to the client to return some server state.
> We reply immediately and we clean the layouts asycnhronously.
>
> FIXME: currently we return _all_ layouts
> FIXME: eventually we should treat layouts as delegations, marked them expired
> and fire the state manager to clean them.
>
> Signed-off-by: Alexandros Batsakis <batsakis@xxxxxxxxxx>
> ---
> fs/nfs/callback.h | 7 ++++++
> fs/nfs/callback_proc.c | 52 +++++++++++++++++++++++++++++++++++++++++++++++-
> 2 files changed, 58 insertions(+), 1 deletions(-)
>
> diff --git a/fs/nfs/callback.h b/fs/nfs/callback.h
> index 73f21bc..b39ac86 100644
> --- a/fs/nfs/callback.h
> +++ b/fs/nfs/callback.h
> @@ -115,6 +115,13 @@ extern int nfs41_validate_delegation_stateid(struct nfs_delegation *delegation,
>
> #define RCA4_TYPE_MASK_RDATA_DLG 0
> #define RCA4_TYPE_MASK_WDATA_DLG 1
> +#define RCA4_TYPE_MASK_DIR_DLG 2
> +#define RCA4_TYPE_MASK_FILE_LAYOUT 3
> +#define RCA4_TYPE_MASK_BLK_LAYOUT 4
> +#define RCA4_TYPE_MASK_OBJ_LAYOUT_MIN 8
> +#define RCA4_TYPE_MASK_OBJ_LAYOUT_MAX 9
> +#define RCA4_TYPE_MASK_OTHER_LAYOUT_MIN 12
> +#define RCA4_TYPE_MASK_OTHER_LAYOUT_MAX 15
>
> struct cb_recallanyargs {
> struct sockaddr *craa_addr;
> diff --git a/fs/nfs/callback_proc.c b/fs/nfs/callback_proc.c
> index 419fee7..d09fb07 100644
> --- a/fs/nfs/callback_proc.c
> +++ b/fs/nfs/callback_proc.c
> @@ -337,6 +337,26 @@ out_put_no_client:
> return status;
> }
>
> +static int pnfs_recall_all_layouts(struct nfs_client *clp)
> +{
> + struct cb_pnfs_layoutrecallargs rl;
> + struct inode *inode;
> + int status = 0;
> +
> + rl.cbl_recall_type = RETURN_ALL;
> + rl.cbl_seg.offset = 0;
> + rl.cbl_seg.length = NFS4_MAX_UINT64;
> + rl.cbl_seg.iomode = IOMODE_ANY;
nit: set the fields in actual memory order...
> + /* we need the inode to get the nfs_server struct */
> + inode = nfs_layoutrecall_find_inode(clp, &rl);
> + if (!inode)
> + return status;
> + status = pnfs_async_return_layout(clp, inode, &rl);
> + iput(inode);
> +
> + return status;
> +}
> +
> __be32 pnfs_cb_layoutrecall(struct cb_pnfs_layoutrecallargs *args,
> void *dummy)
> {
> @@ -651,6 +671,27 @@ out:
> return status;
> }
>
> +static inline int
bool maybe?
> +validate_bitmap_values(const unsigned long* mask)
> +{
> + int i;
> + if (test_bit(RCA4_TYPE_MASK_RDATA_DLG, mask) ||
> + test_bit(RCA4_TYPE_MASK_WDATA_DLG, mask) ||
> + test_bit(RCA4_TYPE_MASK_DIR_DLG, mask) ||
> + test_bit(RCA4_TYPE_MASK_FILE_LAYOUT, mask) ||
> + test_bit(RCA4_TYPE_MASK_BLK_LAYOUT, mask))
> + return 1;
> + for (i = RCA4_TYPE_MASK_OBJ_LAYOUT_MIN;
> + i <= RCA4_TYPE_MASK_OBJ_LAYOUT_MAX; i++)
> + if (test_bit(i, mask))
> + return 1;
> + for (i = RCA4_TYPE_MASK_OTHER_LAYOUT_MIN;
> + i <= RCA4_TYPE_MASK_OTHER_LAYOUT_MAX; i++)
> + if (test_bit(i, mask))
> + return 1;
What about undefined bits?
The spec requires returning NFS4ERR_INVAL in this case.
How about having a mask of all known bits (a constant)
and checking that (mask & ~known_mask) == 0
I'm not absolutely sure that returning INVAL when (mask & known_mask) == 0
(as you effectively do in this patchset) is the right thing to do.
>From the spec it seems like doing nothing might be a better choice.
> + return 0;
> +}
> +
> __be32 nfs4_callback_recallany(struct cb_recallanyargs *args, void *dummy)
> {
> struct nfs_client *clp;
> @@ -665,16 +706,25 @@ __be32 nfs4_callback_recallany(struct cb_recallanyargs *args, void *dummy)
> dprintk("NFS: RECALL_ANY callback request from %s\n",
> rpc_peeraddr2str(clp->cl_rpcclient, RPC_DISPLAY_ADDR));
>
> + status = htonl(NFS4ERR_INVAL);
s/htonl/cpu_to_be32/
> + if (!validate_bitmap_values((const unsigned long *)
> + &args->craa_type_mask))
> + return status;
> +
> if (test_bit(RCA4_TYPE_MASK_RDATA_DLG, (const unsigned long *)
> &args->craa_type_mask))
> flags = FMODE_READ;
> if (test_bit(RCA4_TYPE_MASK_WDATA_DLG, (const unsigned long *)
> &args->craa_type_mask))
> flags |= FMODE_WRITE;
> + if (test_bit(RCA4_TYPE_MASK_FILE_LAYOUT, (const unsigned long *)
> + &args->craa_type_mask))
> + status = pnfs_recall_all_layouts(clp);
this status contains system error not nfs...
>
> if (flags)
> nfs_expire_all_delegation_types(clp, flags);
> - status = htonl(NFS4_OK);
> + if (status != htonl(NFS4ERR_DELAY))
> + status = htonl(NFS4_OK);
ditto
Benny
> out:
> dprintk("%s: exit with status = %d\n", __func__, ntohl(status));
> return status;
--
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
