[pnfs] [PATCH 04/10] pnfs: Use an enum for result of try_to_XXX functions
Benny Halevy
bhalevy at panasas.com
Wed Jul 9 03:26:19 EDT 2008
If you you're going the enum way, then functions that return it
should return enum pnfs_try_status, not "int" so that the compiler
could perform stricter type checking.
On Jul. 09, 2008, 0:54 +0300, Dean Hildebrand <seattleplus at gmail.com> wrote:
> Signed-off-by: Dean Hildebrand <dhildeb at us.ibm.com>
> ---
> fs/nfs/pnfs.c | 16 ++++++++--------
> fs/nfs/pnfs.h | 18 ++++++++++++------
> fs/nfs/read.c | 2 +-
> fs/nfs/write.c | 4 ++--
> 4 files changed, 23 insertions(+), 17 deletions(-)
>
> diff --git a/fs/nfs/pnfs.c b/fs/nfs/pnfs.c
> index 96cf2e0..88a6fa7 100644
> --- a/fs/nfs/pnfs.c
> +++ b/fs/nfs/pnfs.c
> @@ -1497,7 +1497,7 @@ pnfs_writepages(struct nfs_write_data *wdata, int how)
> if (status) {
> dprintk("%s: Updating layout failed (%d), retry with NFS \n",
> __func__, status);
> - status = 1; /* retry with nfs I/O */
> + status = PNFS_NOT_ATTEMPTED; /* retry with nfs I/O */
> goto out;
> }
>
> @@ -1522,7 +1522,7 @@ pnfs_writepages(struct nfs_write_data *wdata, int how)
> how,
> wdata);
>
> - BUG_ON(status < 0);
> + BUG_ON(status < PNFS_ATTEMPTED);
The original condition here was to make sure the LD didn't
return an error (like it used to). With the enum it should
actually be BUG_ON(status != PNFS_ATTEMPTED && status != PNFS_NOT_ATTEMPTED)
I think that if we change its return type we can just get rid of the BUG_ON
and rely on the compiler to warn about this at compile time.
> if (status)
> wdata->pdata.pnfsflags &= ~PNFS_NO_RPC;
> out:
> @@ -1574,7 +1574,7 @@ pnfs_readpages(struct nfs_read_data *rdata)
> if (status) {
> dprintk("%s: ERROR %d from pnfs_update_layout\n",
> __func__, status);
> - status = 1;
> + status = PNFS_NOT_ATTEMPTED;
> goto out;
> }
>
> @@ -1598,7 +1598,7 @@ pnfs_readpages(struct nfs_read_data *rdata)
> (loff_t)args->offset,
> args->count,
> rdata);
> - BUG_ON(status < 0);
> + BUG_ON(status < PNFS_ATTEMPTED);
same here.
> if (status)
> rdata->pdata.pnfsflags &= ~PNFS_NO_RPC;
> out:
> @@ -1617,7 +1617,7 @@ int _pnfs_try_to_read_data(struct nfs_read_data *data,
> if (!pnfs_use_read(ino, data->args.count) ||
> !nfss->pnfs_curr_ld->ld_io_ops->read_pagelist) {
> dprintk("<-- %s: not using pnfs\n", __func__);
> - return 1;
> + return PNFS_NOT_ATTEMPTED;
> } else {
> dprintk("%s: Utilizing pNFS I/O\n", __func__);
> data->pdata.call_ops = call_ops;
> @@ -1703,7 +1703,7 @@ int _pnfs_try_to_write_data(struct nfs_write_data *data,
> if (!pnfs_use_write(ino, data->args.count) ||
> !nfss->pnfs_curr_ld->ld_io_ops->write_pagelist) {
> dprintk("<-- %s: not using pnfs\n", __func__);
> - return 1;
> + return PNFS_NOT_ATTEMPTED;
> } else {
> dprintk("%s: Utilizing pNFS I/O\n", __func__);
> data->pdata.call_ops = call_ops;
> @@ -1720,7 +1720,7 @@ int _pnfs_try_to_commit(struct nfs_write_data *data,
>
> if (!pnfs_use_write(inode, -1)) {
> dprintk("%s: Not using pNFS I/O\n", __func__);
> - return 1;
> + return PNFS_NOT_ATTEMPTED;
> } else {
> /* data->call_ops and data->how set in nfs_commit_rpcsetup */
> dprintk("%s: Utilizing pNFS I/O\n", __func__);
> @@ -1785,7 +1785,7 @@ pnfs_commit(struct nfs_write_data *data, int sync)
> * and o_direct commit processing.
> */
> dprintk("%s: no layout. Not using pNFS.\n", __func__);
> - return 1;
> + return PNFS_NOT_ATTEMPTED;
> }
>
> dprintk("%s: Calling layout driver commit\n", __func__);
> diff --git a/fs/nfs/pnfs.h b/fs/nfs/pnfs.h
> index 340ebb3..e2e32d9 100644
> --- a/fs/nfs/pnfs.h
> +++ b/fs/nfs/pnfs.h
> @@ -16,7 +16,13 @@
> #include <linux/nfs_page.h>
> #include <linux/nfs4_pnfs.h>
>
> +enum pnfs_try_status {
> + PNFS_ATTEMPTED = 0,
> + PNFS_NOT_ATTEMPTED = 1,
> +};
> +
> #ifdef CONFIG_PNFS
> +
> /* nfs4proc.c */
> extern int nfs4_pnfs_getdevicelist(struct super_block *sb, struct nfs_fh *fh,
> struct pnfs_devicelist *devlist);
> @@ -92,7 +98,7 @@ static inline int pnfs_try_to_read_data(struct nfs_read_data *data,
> if (PNFS_EXISTS_LDIO_OP(nfss, read_pagelist))
> return _pnfs_try_to_read_data(data, call_ops);
>
> - return 1;
> + return PNFS_NOT_ATTEMPTED;
> }
>
> static inline int pnfs_try_to_write_data(struct nfs_write_data *data,
> @@ -106,7 +112,7 @@ static inline int pnfs_try_to_write_data(struct nfs_write_data *data,
> if (PNFS_EXISTS_LDIO_OP(nfss, write_pagelist))
> return _pnfs_try_to_write_data(data, call_ops, how);
>
> - return 1;
> + return PNFS_NOT_ATTEMPTED;
> }
>
> static inline int pnfs_try_to_commit(struct nfs_write_data *data,
> @@ -123,7 +129,7 @@ static inline int pnfs_try_to_commit(struct nfs_write_data *data,
> if (PNFS_EXISTS_LDIO_OP(nfss, write_pagelist))
> return _pnfs_try_to_commit(data, call_ops, how);
>
> - return 1;
> + return PNFS_NOT_ATTEMPTED;
> }
>
> static inline int pnfs_write_begin(struct file *filp, struct page *page,
> @@ -207,21 +213,21 @@ static inline int pnfs_return_layout(struct inode *ino,
> static inline int pnfs_try_to_read_data(struct nfs_read_data *data,
> const struct rpc_call_ops *call_ops)
> {
> - return 1;
> + return PNFS_NOT_ATTEMPTED;
> }
>
> static inline int pnfs_try_to_write_data(struct nfs_write_data *data,
> const struct rpc_call_ops *call_ops,
> int how)
> {
> - return 1;
> + return PNFS_NOT_ATTEMPTED;
> }
>
> static inline int pnfs_try_to_commit(struct nfs_write_data *data,
> const struct rpc_call_ops *call_ops,
> int how)
> {
> - return 1;
> + return PNFS_NOT_ATTEMPTED;
> }
>
> static inline int pnfs_do_flush(struct nfs_page *req, void *fsdata)
> diff --git a/fs/nfs/read.c b/fs/nfs/read.c
> index 31b4c40..6f96a53 100644
> --- a/fs/nfs/read.c
> +++ b/fs/nfs/read.c
> @@ -224,7 +224,7 @@ static int nfs_read_rpcsetup(struct nfs_page *req, struct nfs_read_data *data,
>
> ret = pnfs_try_to_read_data(data, call_ops);
> #if defined(CONFIG_PNFS)
> - if (ret == 0)
> + if (ret == PNFS_ATTEMPTED)
> return data->pdata.pnfs_error;
> #endif /* CONFIG_PNFS */
>
> diff --git a/fs/nfs/write.c b/fs/nfs/write.c
> index fbffb57..b5e046f 100644
> --- a/fs/nfs/write.c
> +++ b/fs/nfs/write.c
> @@ -868,7 +868,7 @@ static int nfs_write_rpcsetup(struct nfs_page *req,
>
> ret = pnfs_try_to_write_data(data, call_ops, how);
> #if defined(CONFIG_PNFS)
> - if (ret == 0)
> + if (ret == PNFS_ATTEMPTED)
> return data->pdata.pnfs_error;
> #endif /* CONFIG_PNFS */
>
> @@ -1313,7 +1313,7 @@ static int nfs_commit_rpcsetup(struct list_head *head,
>
> data->args.context = first->wb_context; /* used by commit done */
> ret = pnfs_try_to_commit(data, &nfs_commit_ops, how);
> - if (ret <= 0)
> + if (ret <= PNFS_ATTEMPTED)
> return ret;
>
> return nfs_initiate_commit(data, NFS_CLIENT(inode), how);
More information about the pNFS
mailing list