Received-SPF: pass (google.com: domain of linux-nfs-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
From:   Chuck Lever <chuck.lever@oracle.com>
To:     Roberto Bergantinos Corpas <rbergant@redhat.com>
CC:     Trond Myklebust <trond.myklebust@hammerspace.com>,
        Anna Schumaker <anna.schumaker@netapp.com>,
        Linux NFS Mailing List <linux-nfs@vger.kernel.org>
Subject: Re: [PATCH] pNFS: make DS availability problems visible in log
Thread-Topic: [PATCH] pNFS: make DS availability problems visible in log
Thread-Index: AQHXEJDleBWe0KC7fkOoSFm+lqhgjKp0RoAA
Date:   Thu, 4 Mar 2021 20:20:29 +0000
Message-ID: <FBBBDFDD-6819-450C-879D-0B11B917BD10@oracle.com>
References: <20210303153307.3147-1-rbergant@redhat.com>
In-Reply-To: <20210303153307.3147-1-rbergant@redhat.com>
Accept-Language: en-US
Content-Language: en-US
Content-Type: text/plain; charset="us-ascii"
Content-ID: <E49196300D6B69488EA1E2F3C958A3AA@namprd10.prod.outlook.com>
Content-Transfer-Encoding: quoted-printable
MIME-Version: 1.0
X-MS-Exchange-CrossTenant-Network-Message-Id: 73602955-68ae-41f3-08dc-08d8df4af488
X-MS-Exchange-CrossTenant-originalarrivaltime: 04 Mar 2021 20:20:29.4830
 (UTC)
X-MS-Exchange-CrossTenant-fromentityheader: Hosted
X-MS-Exchange-CrossTenant-id: 4e2c6054-71cb-48f1-bd6c-3a9705aca71b
X-MS-Exchange-CrossTenant-mailboxtype: HOSTED
X-MS-Exchange-CrossTenant-userprincipalname: 8Ljnvd2kAdk54mIt7X3XnE0fRpziZndqCu1z18MTMR72v0/PIkVNode6w6p8XCHSI+IfyU/bA0y/lqK6dVdQaA==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: BYAPR10MB3110
X-Proofpoint-Virus-Version: vendor=nai engine=6200 definitions=9913 signatures=668683
X-Proofpoint-Spam-Details: rule=notspam policy=default score=0 adultscore=0 mlxscore=0 spamscore=0
 bulkscore=0 suspectscore=0 mlxlogscore=999 phishscore=0 malwarescore=0
 classifier=spam adjust=0 reason=mlx scancount=1 engine=8.12.0-2009150000
 definitions=main-2103040098
X-Proofpoint-Virus-Version: vendor=nai engine=6200 definitions=9913 signatures=668683
X-Proofpoint-Spam-Details: rule=notspam policy=default score=0 lowpriorityscore=0 clxscore=1011
 priorityscore=1501 mlxlogscore=999 suspectscore=0 malwarescore=0
 impostorscore=0 bulkscore=0 adultscore=0 mlxscore=0 phishscore=0
 spamscore=0 classifier=spam adjust=0 reason=mlx scancount=1
 engine=8.12.0-2009150000 definitions=main-2103040099
Precedence: bulk
List-ID: <linux-nfs.vger.kernel.org>
X-Mailing-List: linux-nfs@vger.kernel.org

Hello Roberto!

> On Mar 3, 2021, at 10:33 AM, Roberto Bergantinos Corpas <rbergant@redhat.=
com> wrote:
>=20
> Would be interesting to promote DS availability logging outside debug
> so that we are more aware that I/O is diverted to MDS and some part
> of the infraestructure failed.
>=20
> Also added logging for failed DS connection attempts.

Given that this enables remote system behavior to generate
kernel log traffic that can fill the local root partition,
I'd like to see either:

- the explicit use of rate limiting, or

- these dprintks replaced with tracepoints


> Signed-off-by: Roberto Bergantinos Corpas <rbergant@redhat.com>
> ---
> fs/nfs/filelayout/filelayout.c         | 4 ++--
> fs/nfs/flexfilelayout/flexfilelayout.c | 6 +++---
> fs/nfs/pnfs_nfs.c                      | 6 +++++-
> 3 files changed, 10 insertions(+), 6 deletions(-)
>=20
> diff --git a/fs/nfs/filelayout/filelayout.c b/fs/nfs/filelayout/filelayou=
t.c
> index 7f5aa0403e16..fef2d31a501a 100644
> --- a/fs/nfs/filelayout/filelayout.c
> +++ b/fs/nfs/filelayout/filelayout.c
> @@ -181,7 +181,7 @@ static int filelayout_async_handle_error(struct rpc_t=
ask *task,
> 	case -EIO:
> 	case -ETIMEDOUT:
> 	case -EPIPE:
> -		dprintk("%s DS connection error %d\n", __func__,
> +		pr_warn("%s DS connection error %d\n", __func__,
> 			task->tk_status);
> 		nfs4_mark_deviceid_unavailable(devid);
> 		pnfs_error_mark_layout_for_return(inode, lseg);
> @@ -190,7 +190,7 @@ static int filelayout_async_handle_error(struct rpc_t=
ask *task,
> 		fallthrough;
> 	default:
> reset:
> -		dprintk("%s Retry through MDS. Error %d\n", __func__,
> +		pr_warn("%s Retry through MDS. Error %d\n", __func__,
> 			task->tk_status);
> 		return -NFS4ERR_RESET_TO_MDS;
> 	}
> diff --git a/fs/nfs/flexfilelayout/flexfilelayout.c b/fs/nfs/flexfilelayo=
ut/flexfilelayout.c
> index a163533446fa..7150d94e80e6 100644
> --- a/fs/nfs/flexfilelayout/flexfilelayout.c
> +++ b/fs/nfs/flexfilelayout/flexfilelayout.c
> @@ -1129,7 +1129,7 @@ static int ff_layout_async_handle_error_v4(struct r=
pc_task *task,
> 	case -EIO:
> 	case -ETIMEDOUT:
> 	case -EPIPE:
> -		dprintk("%s DS connection error %d\n", __func__,
> +		pr_warn("%s DS connection error %d\n", __func__,
> 			task->tk_status);
> 		nfs4_delete_deviceid(devid->ld, devid->nfs_client,
> 				&devid->deviceid);
> @@ -1139,7 +1139,7 @@ static int ff_layout_async_handle_error_v4(struct r=
pc_task *task,
> 		if (ff_layout_avoid_mds_available_ds(lseg))
> 			return -NFS4ERR_RESET_TO_PNFS;
> reset:
> -		dprintk("%s Retry through MDS. Error %d\n", __func__,
> +		pr_warn("%s Retry through MDS. Error %d\n", __func__,
> 			task->tk_status);
> 		return -NFS4ERR_RESET_TO_MDS;
> 	}
> @@ -1167,7 +1167,7 @@ static int ff_layout_async_handle_error_v3(struct r=
pc_task *task,
> 		nfs_inc_stats(lseg->pls_layout->plh_inode, NFSIOS_DELAY);
> 		goto out_retry;
> 	default:
> -		dprintk("%s DS connection error %d\n", __func__,
> +		pr_warn("%s DS connection error %d\n", __func__,
> 			task->tk_status);
> 		nfs4_delete_deviceid(devid->ld, devid->nfs_client,
> 				&devid->deviceid);
> diff --git a/fs/nfs/pnfs_nfs.c b/fs/nfs/pnfs_nfs.c
> index 679767ac258d..322661a48348 100644
> --- a/fs/nfs/pnfs_nfs.c
> +++ b/fs/nfs/pnfs_nfs.c
> @@ -934,8 +934,11 @@ static int _nfs4_pnfs_v4_ds_connect(struct nfs_serve=
r *mds_srv,
> 						(struct sockaddr *)&da->da_addr,
> 						da->da_addrlen, IPPROTO_TCP,
> 						timeo, retrans, minor_version);
> -			if (IS_ERR(clp))
> +			if (IS_ERR(clp)) {
> +				pr_warn("%s: DS: %s unable to connect with address %s, error: %ld\n"=
,
> +					__func__, ds->ds_remotestr, da->da_remotestr, PTR_ERR(clp));
> 				continue;
> +			}
>=20
> 			status =3D nfs4_init_ds_session(clp,
> 					mds_srv->nfs_client->cl_lease_time);
> @@ -949,6 +952,7 @@ static int _nfs4_pnfs_v4_ds_connect(struct nfs_server=
 *mds_srv,
> 	}
>=20
> 	if (IS_ERR(clp)) {
> +		pr_warn("%s: no DS available\n", __func__);
> 		status =3D PTR_ERR(clp);
> 		goto out;
> 	}
> --=20
> 2.21.0
>=20

--
Chuck Lever