All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 1/1] NFSD: Fix problem with nfsd4_scsi_fence_client using the wrong reservation type
@ 2025-10-29 23:28 Dai Ngo
  2025-10-30  5:55 ` Christoph Hellwig
  2025-10-30 13:33 ` Chuck Lever
  0 siblings, 2 replies; 4+ messages in thread
From: Dai Ngo @ 2025-10-29 23:28 UTC (permalink / raw)
  To: chuck.lever, jlayton, neilb, okorniev, tom, hch; +Cc: linux-nfs

The reservation type argument for the pr_preempt call should match the
one used in nfsd4_block_get_device_info_scsi. Additionally, the pr_preempt
operation only needs to be executed once per SCSI target.

Signed-off-by: Dai Ngo <dai.ngo@oracle.com>
---
 fs/nfsd/blocklayout.c | 17 +++++++++++++++--
 fs/nfsd/state.h       |  1 +
 2 files changed, 16 insertions(+), 2 deletions(-)

diff --git a/fs/nfsd/blocklayout.c b/fs/nfsd/blocklayout.c
index fde5539cf6a6..4ca6cb420736 100644
--- a/fs/nfsd/blocklayout.c
+++ b/fs/nfsd/blocklayout.c
@@ -340,9 +340,22 @@ nfsd4_scsi_fence_client(struct nfs4_layout_stateid *ls, struct nfsd_file *file)
 {
 	struct nfs4_client *clp = ls->ls_stid.sc_client;
 	struct block_device *bdev = file->nf_file->f_path.mnt->mnt_sb->s_bdev;
+	int error;
+
+	if (ls->ls_fenced)
+		return;
+	ls->ls_fenced = true;
+	error = bdev->bd_disk->fops->pr_ops->pr_preempt(bdev, NFSD_MDS_PR_KEY,
+			nfsd4_scsi_pr_key(clp),
+			PR_EXCLUSIVE_ACCESS_REG_ONLY, true);
+	if (error) {
+		char addr_str[INET6_ADDRSTRLEN];
 
-	bdev->bd_disk->fops->pr_ops->pr_preempt(bdev, NFSD_MDS_PR_KEY,
-			nfsd4_scsi_pr_key(clp), 0, true);
+		ls->ls_fenced = false;
+		rpc_ntop((struct sockaddr *)&clp->cl_addr, addr_str, sizeof(addr_str));
+		dprintk("nfsd: failed to fence client %s error %d\n",
+			addr_str, error);
+	}
 }
 
 const struct nfsd4_layout_ops scsi_layout_ops = {
diff --git a/fs/nfsd/state.h b/fs/nfsd/state.h
index 1e736f402426..1de4acc7d539 100644
--- a/fs/nfsd/state.h
+++ b/fs/nfsd/state.h
@@ -735,6 +735,7 @@ struct nfs4_layout_stateid {
 	stateid_t			ls_recall_sid;
 	bool				ls_recalled;
 	struct mutex			ls_mutex;
+	bool				ls_fenced;
 };
 
 static inline struct nfs4_layout_stateid *layoutstateid(struct nfs4_stid *s)
-- 
2.47.3


^ permalink raw reply related	[flat|nested] 4+ messages in thread

* Re: [PATCH 1/1] NFSD: Fix problem with nfsd4_scsi_fence_client using the wrong reservation type
  2025-10-29 23:28 [PATCH 1/1] NFSD: Fix problem with nfsd4_scsi_fence_client using the wrong reservation type Dai Ngo
@ 2025-10-30  5:55 ` Christoph Hellwig
  2025-10-30 17:25   ` Dai Ngo
  2025-10-30 13:33 ` Chuck Lever
  1 sibling, 1 reply; 4+ messages in thread
From: Christoph Hellwig @ 2025-10-30  5:55 UTC (permalink / raw)
  To: Dai Ngo; +Cc: chuck.lever, jlayton, neilb, okorniev, tom, hch, linux-nfs

On Wed, Oct 29, 2025 at 04:28:26PM -0700, Dai Ngo wrote:
> The reservation type argument for the pr_preempt call should match the
> one used in nfsd4_block_get_device_info_scsi. Additionally, the pr_preempt
> operation only needs to be executed once per SCSI target.

One patch per change, please.

Also please add a Fixes: tag.

> 
> Signed-off-by: Dai Ngo <dai.ngo@oracle.com>
> ---
>  fs/nfsd/blocklayout.c | 17 +++++++++++++++--
>  fs/nfsd/state.h       |  1 +
>  2 files changed, 16 insertions(+), 2 deletions(-)
> 
> diff --git a/fs/nfsd/blocklayout.c b/fs/nfsd/blocklayout.c
> index fde5539cf6a6..4ca6cb420736 100644
> --- a/fs/nfsd/blocklayout.c
> +++ b/fs/nfsd/blocklayout.c
> @@ -340,9 +340,22 @@ nfsd4_scsi_fence_client(struct nfs4_layout_stateid *ls, struct nfsd_file *file)
>  {
>  	struct nfs4_client *clp = ls->ls_stid.sc_client;
>  	struct block_device *bdev = file->nf_file->f_path.mnt->mnt_sb->s_bdev;
> +	int error;
> +
> +	if (ls->ls_fenced)
> +		return;
> +	ls->ls_fenced = true;

What is used to serialize this?

> +		ls->ls_fenced = false;
> +		rpc_ntop((struct sockaddr *)&clp->cl_addr, addr_str, sizeof(addr_str));

Overly long line.

> +		dprintk("nfsd: failed to fence client %s error %d\n",
> +			addr_str, error);
> +	}

Also the error printing is new without any rationale in the commit
message.


^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH 1/1] NFSD: Fix problem with nfsd4_scsi_fence_client using the wrong reservation type
  2025-10-29 23:28 [PATCH 1/1] NFSD: Fix problem with nfsd4_scsi_fence_client using the wrong reservation type Dai Ngo
  2025-10-30  5:55 ` Christoph Hellwig
@ 2025-10-30 13:33 ` Chuck Lever
  1 sibling, 0 replies; 4+ messages in thread
From: Chuck Lever @ 2025-10-30 13:33 UTC (permalink / raw)
  To: Dai Ngo, jlayton, neilb, okorniev, tom, hch; +Cc: linux-nfs

On 10/29/25 7:28 PM, Dai Ngo wrote:
> The reservation type argument for the pr_preempt call should match the
> one used in nfsd4_block_get_device_info_scsi. Additionally, the pr_preempt
> operation only needs to be executed once per SCSI target.

As hch mentioned, let's keep each of these two fixes in their own patch
with appropriate Fixes tags for both.


> Signed-off-by: Dai Ngo <dai.ngo@oracle.com>
> ---
>  fs/nfsd/blocklayout.c | 17 +++++++++++++++--
>  fs/nfsd/state.h       |  1 +
>  2 files changed, 16 insertions(+), 2 deletions(-)
> 
> diff --git a/fs/nfsd/blocklayout.c b/fs/nfsd/blocklayout.c
> index fde5539cf6a6..4ca6cb420736 100644
> --- a/fs/nfsd/blocklayout.c
> +++ b/fs/nfsd/blocklayout.c
> @@ -340,9 +340,22 @@ nfsd4_scsi_fence_client(struct nfs4_layout_stateid *ls, struct nfsd_file *file)
>  {
>  	struct nfs4_client *clp = ls->ls_stid.sc_client;
>  	struct block_device *bdev = file->nf_file->f_path.mnt->mnt_sb->s_bdev;
> +	int error;
> +
> +	if (ls->ls_fenced)
> +		return;
> +	ls->ls_fenced = true;
> +	error = bdev->bd_disk->fops->pr_ops->pr_preempt(bdev, NFSD_MDS_PR_KEY,
> +			nfsd4_scsi_pr_key(clp),
> +			PR_EXCLUSIVE_ACCESS_REG_ONLY, true);
> +	if (error) {
> +		char addr_str[INET6_ADDRSTRLEN];
>  
> -	bdev->bd_disk->fops->pr_ops->pr_preempt(bdev, NFSD_MDS_PR_KEY,
> -			nfsd4_scsi_pr_key(clp), 0, true);
> +		ls->ls_fenced = false;
> +		rpc_ntop((struct sockaddr *)&clp->cl_addr, addr_str, sizeof(addr_str));
> +		dprintk("nfsd: failed to fence client %s error %d\n",
> +			addr_str, error);

Instead of a dprintk call site, would a new trace point be more
appropriate?


> +	}
>  }
>  
>  const struct nfsd4_layout_ops scsi_layout_ops = {
> diff --git a/fs/nfsd/state.h b/fs/nfsd/state.h
> index 1e736f402426..1de4acc7d539 100644
> --- a/fs/nfsd/state.h
> +++ b/fs/nfsd/state.h
> @@ -735,6 +735,7 @@ struct nfs4_layout_stateid {
>  	stateid_t			ls_recall_sid;
>  	bool				ls_recalled;
>  	struct mutex			ls_mutex;
> +	bool				ls_fenced;
>  };
>  
>  static inline struct nfs4_layout_stateid *layoutstateid(struct nfs4_stid *s)


-- 
Chuck Lever

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH 1/1] NFSD: Fix problem with nfsd4_scsi_fence_client using the wrong reservation type
  2025-10-30  5:55 ` Christoph Hellwig
@ 2025-10-30 17:25   ` Dai Ngo
  0 siblings, 0 replies; 4+ messages in thread
From: Dai Ngo @ 2025-10-30 17:25 UTC (permalink / raw)
  To: Christoph Hellwig; +Cc: chuck.lever, jlayton, okorniev, tom, linux-nfs, neilb

Thank you for your review!

On 10/29/25 10:55 PM, Christoph Hellwig wrote:
> On Wed, Oct 29, 2025 at 04:28:26PM -0700, Dai Ngo wrote:
>> The reservation type argument for the pr_preempt call should match the
>> one used in nfsd4_block_get_device_info_scsi. Additionally, the pr_preempt
>> operation only needs to be executed once per SCSI target.
> One patch per change, please.
>
> Also please add a Fixes: tag.

will fix.

>
>> Signed-off-by: Dai Ngo <dai.ngo@oracle.com>
>> ---
>>   fs/nfsd/blocklayout.c | 17 +++++++++++++++--
>>   fs/nfsd/state.h       |  1 +
>>   2 files changed, 16 insertions(+), 2 deletions(-)
>>
>> diff --git a/fs/nfsd/blocklayout.c b/fs/nfsd/blocklayout.c
>> index fde5539cf6a6..4ca6cb420736 100644
>> --- a/fs/nfsd/blocklayout.c
>> +++ b/fs/nfsd/blocklayout.c
>> @@ -340,9 +340,22 @@ nfsd4_scsi_fence_client(struct nfs4_layout_stateid *ls, struct nfsd_file *file)
>>   {
>>   	struct nfs4_client *clp = ls->ls_stid.sc_client;
>>   	struct block_device *bdev = file->nf_file->f_path.mnt->mnt_sb->s_bdev;
>> +	int error;
>> +
>> +	if (ls->ls_fenced)
>> +		return;
>> +	ls->ls_fenced = true;
> What is used to serialize this?

ok, I'll use the ls_lock here.

>
>> +		ls->ls_fenced = false;
>> +		rpc_ntop((struct sockaddr *)&clp->cl_addr, addr_str, sizeof(addr_str));
> Overly long line.

will fix.

>
>> +		dprintk("nfsd: failed to fence client %s error %d\n",
>> +			addr_str, error);
>> +	}
> Also the error printing is new without any rationale in the commit
> message.

As Chuck suggested, I will add a trace point here in a separate patch.

-Dai

>

^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2025-10-30 17:26 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-10-29 23:28 [PATCH 1/1] NFSD: Fix problem with nfsd4_scsi_fence_client using the wrong reservation type Dai Ngo
2025-10-30  5:55 ` Christoph Hellwig
2025-10-30 17:25   ` Dai Ngo
2025-10-30 13:33 ` Chuck Lever

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.