On Tue, 2012-08-07 at 11:30 -0400, bjschuma@xxxxxxxxxx wrote:
> From: Bryan Schumaker <bjschuma@xxxxxxxxxx>
>
> idmap_pipe_downcall already clears this field if the upcall succeeds,
> but if it fails (rpc.idmapd isn't running) the field will still be set
> on the next call triggering a BUG_ON().
>
> Signed-off-by: Bryan Schumaker <bjschuma@xxxxxxxxxx>
> ---
> fs/nfs/idmap.c | 16 +++++++++-------
> 1 file changed, 9 insertions(+), 7 deletions(-)
>
> diff --git a/fs/nfs/idmap.c b/fs/nfs/idmap.c
> index b701358..645cfe7 100644
> --- a/fs/nfs/idmap.c
> +++ b/fs/nfs/idmap.c
> @@ -683,10 +683,12 @@ static int nfs_idmap_legacy_upcall(struct key_construction *cons,
>
> ret = rpc_queue_upcall(idmap->idmap_pipe, msg);
> if (ret < 0)
> - goto out2;
> + goto out3;
>
> return ret;
>
> +out3:
> + idmap->idmap_key_cons = NULL;
> out2:
> kfree(im);
> out1:
That fixes rpc_queue_upcall failure path, but we still need to deal with
timeouts and close.
How about the following alternative:
Set idmap->idmap_key_cons after grabbing the mutex in
nfs_idmap_get_key(), then clear it before you release that mutex. That
leaves one possible race in which the idmap->idmap_key_cons is cleared
while we're in idmap_pipe_downcall. One way to solve that race is to use
a second mutex (see the original idmapper design from before your
changes) to ensure that nothing clears idmap_key_cons while we're in
idmap_pipe_downcall.
--
Trond Myklebust
Linux NFS client maintainer
NetApp
Trond.Myklebust@xxxxxxxxxx
www.netapp.com
��.n��������+%������w��{.n�����{��w��� jg��� �����ݢj����G���� ���j:+v���w�m������w�������h�����٥
