archive mirror
 help / color / mirror / Atom feed
From: Jason Gunthorpe <jgunthorpe-ePGOBjL8dl3ta4EC/59zMFaTQe2KTcn/>
To: Alexander Steffen
Subject: Re: [PATCH 1/3] tpm-chip: Move idr_replace calls to appropriate places
Date: Sun, 9 Jul 2017 15:24:30 -0600	[thread overview]
Message-ID: <> (raw)
In-Reply-To: <20170704135648.7360-1-Alexander.Steffen-d0qZbvYSIPpWk0Htik3J/>

On Tue, Jul 04, 2017 at 03:56:46PM +0200, Alexander Steffen wrote:
> According to the comments, adding/removing the chip from the list should be
> the first/last action in (un)register.

The comments are misleading..

> diff --git a/drivers/char/tpm/tpm-chip.c b/drivers/char/tpm/tpm-chip.c
> index 67ec9d3..a353b7a 100644
> +++ b/drivers/char/tpm/tpm-chip.c
> @@ -327,11 +327,6 @@ static int tpm_add_char_device(struct tpm_chip *chip)
>  		}
>  	}
> -	/* Make the chip available. */
> -	mutex_lock(&idr_lock);
> -	idr_replace(&dev_nums_idr, chip, chip->dev_num);
> -	mutex_unlock(&idr_lock);

This is actually in the wrong place already, it needs to be done
before cdev_device_add - this is because cdev_device_add generates the
uevent to userspace which could trigger userspace to use the kernel
device. So a patch to move it to the start of this function woud
be good. The function would be better described as 'make visible'

Maybe resend this patch with only that change..

>  {
>  	cdev_device_del(&chip->cdev, &chip->dev);
> -	/* Make the chip unavailable. */
> -	mutex_lock(&idr_lock);
> -	idr_replace(&dev_nums_idr, NULL, chip->dev_num);
> -	mutex_unlock(&idr_lock);
> -

The placement of this does not matter so much, but keeping it after
the cdev_device_del is easier to understand as it matches the
(corrected) setup order..


Check out the vibrant tech community on one of the world's most
engaging tech sites,!

  parent reply	other threads:[~2017-07-09 21:24 UTC|newest]

Thread overview: 5+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-07-04 13:56 Alexander Steffen
     [not found] ` <20170704135648.7360-1-Alexander.Steffen-d0qZbvYSIPpWk0Htik3J/>
2017-07-04 13:56   ` [PATCH 2/3] tpm-chip: Return TPM error codes from auto_startup functions Alexander Steffen
2017-07-04 13:56   ` [PATCH 3/3] tpm-chip: Export TPM device to user space even when startup failed Alexander Steffen
2017-07-09 21:24   ` Jason Gunthorpe [this message]
     [not found]     ` <20170709212430.GB19327-ePGOBjL8dl3ta4EC/59zMFaTQe2KTcn/>
2017-07-27 14:07       ` [PATCH 1/3] tpm-chip: Move idr_replace calls to appropriate places Alexander.Steffen-d0qZbvYSIPpWk0Htik3J/w

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \ \
    --to=jgunthorpe-epgobjl8dl3ta4ec/59zmfatqe2ktcn/ \
    --cc=Alexander.Steffen-d0qZbvYSIPpWk0Htik3J/ \ \
    --subject='Re: [PATCH 1/3] tpm-chip: Move idr_replace calls to appropriate places' \

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).