On Thu, Jun 20, 2024 at 11:45:02AM -0400, Benjamin Coddington wrote:
> On 20 Jun 2024, at 10:15, Christoph Hellwig wrote:
>
> > On Thu, Jun 20, 2024 at 09:52:59AM -0400, Benjamin Coddington wrote:
> >> On 20 Jun 2024, at 1:06, Christoph Hellwig wrote:
> >>
> >>> On Wed, Jun 19, 2024 at 01:39:33PM -0400, cel@xxxxxxxxxx wrote:
> >>>> - if (test_bit(NFS_DEVICEID_UNAVAILABLE, &node->flags) == 0)
> >>>> + if (test_bit(NFS_DEVICEID_UNAVAILABLE, &node->flags) == 0) {
> >>>
> >>> It might be worth to invert this and keep the unavailable handling in
> >>> the branch as that's the exceptional case. That code is also woefully
> >>> under-documented and could have really used a comment.
> >>
> >> The transient device handling in general, or just this bit of it?
> >
> > Basically the code behind this NFS_DEVICEID_UNAVAILABLE check here.
>
> How about..
Let's leave this as a separate patch. IMO this is dealing with an
entirely orthogonal issue.
> diff --git a/fs/nfs/blocklayout/blocklayout.c b/fs/nfs/blocklayout/blocklayout.c
> index 6be13e0ec170..665c054784b4 100644
> --- a/fs/nfs/blocklayout/blocklayout.c
> +++ b/fs/nfs/blocklayout/blocklayout.c
> @@ -564,25 +564,30 @@ bl_find_get_deviceid(struct nfs_server *server,
> gfp_t gfp_mask)
> {
> struct nfs4_deviceid_node *node;
> - unsigned long start, end;
>
> retry:
> node = nfs4_find_get_deviceid(server, id, cred, gfp_mask);
> if (!node)
> return ERR_PTR(-ENODEV);
>
> - if (test_bit(NFS_DEVICEID_UNAVAILABLE, &node->flags) == 0)
> - return node;
> + /* Transient devices are left in the cache with a timeout to minimize
> + * sending GETDEVINFO after every LAYOUTGET */
> + if (test_bit(NFS_DEVICEID_UNAVAILABLE, &node->flags)) {
> + unsigned long start, end;
>
> - end = jiffies;
> - start = end - PNFS_DEVICE_RETRY_TIMEOUT;
> - if (!time_in_range(node->timestamp_unavailable, start, end)) {
> - nfs4_delete_deviceid(node->ld, node->nfs_client, id);
> - goto retry;
> + end = jiffies;
> + start = end - PNFS_DEVICE_RETRY_TIMEOUT;
> +
> + /* Maybe the device is back, let's look for it again */
> + if (!time_in_range(node->timestamp_unavailable, start, end)) {
> + nfs4_delete_deviceid(node->ld, node->nfs_client, id);
> + goto retry;
> + }
> + nfs4_put_deviceid_node(node);
> + return ERR_PTR(-ENODEV);
> }
>
> - nfs4_put_deviceid_node(node);
> - return ERR_PTR(-ENODEV);
> + return node;
> }
>
> static int
>
--
Chuck Lever
