On Wed, Aug 09, 2017 at 03:09:18PM +0200, Hannes Reinecke wrote:
> While the 'state' attribute can (and will) change occasionally,
> calling 'poll()' or 'select()' on it fails as sysfs is never
> notified that the state has changed.
> With this patch calling 'poll()' or 'select()' will work
> properly.
> 
> Signed-off-by: Hannes Reinecke <h...@suse.com>
> ---
>  drivers/scsi/scsi_lib.c           | 7 +++++--
>  drivers/scsi/scsi_transport_srp.c | 5 ++++-
>  2 files changed, 9 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/scsi/scsi_lib.c b/drivers/scsi/scsi_lib.c
> index 41c19c7..2101cfd 100644
> --- a/drivers/scsi/scsi_lib.c
> +++ b/drivers/scsi/scsi_lib.c
> @@ -2654,6 +2654,7 @@ void scsi_exit_queue(void)
>  
>       }
>       sdev->sdev_state = state;
> +     sysfs_notify(&sdev->sdev_gendev.kobj, NULL, "state");
>       return 0;
>  
>   illegal:
> @@ -3074,14 +3075,16 @@ int scsi_internal_device_unblock_nowait(struct 
> scsi_device *sdev,
>        * offlined states and goose the device queue if successful.
>        */
>       if ((sdev->sdev_state == SDEV_BLOCK) ||
> -         (sdev->sdev_state == SDEV_TRANSPORT_OFFLINE))
> +         (sdev->sdev_state == SDEV_TRANSPORT_OFFLINE)) {
>               sdev->sdev_state = new_state;
> -     else if (sdev->sdev_state == SDEV_CREATED_BLOCK) {
> +             sysfs_notify(&sdev->sdev_gendev.kobj, NULL, "state");
> +     } else if (sdev->sdev_state == SDEV_CREATED_BLOCK) {
>               if (new_state == SDEV_TRANSPORT_OFFLINE ||
>                   new_state == SDEV_OFFLINE)
>                       sdev->sdev_state = new_state;
>               else
>                       sdev->sdev_state = SDEV_CREATED;
> +             sysfs_notify(&sdev->sdev_gendev.kobj, NULL, "state");
>       } else if (sdev->sdev_state != SDEV_CANCEL &&
>                sdev->sdev_state != SDEV_OFFLINE)
>               return -EINVAL;

This would be a lot more readable using switch statements:

        switch (sdev->sdev_state) {
        case SDEV_BLOCK:
        case SDEV_TRANSPORT_OFFLINE:
                sdev->sdev_state = new_state;
                sysfs_notify(&sdev->sdev_gendev.kobj, NULL, "state");
                break;
        case SDEV_CREATED_BLOCK:
                switch (new_state) {
                case SDEV_TRANSPORT_OFFLINE:
                case SDEV_OFFLINE:
                        sdev->sdev_state = new_state;
                        break;
                default:
                        sdev->sdev_state = SDEV_CREATED;
                        break;
                }
                sysfs_notify(&sdev->sdev_gendev.kobj, NULL, "state");
                break;
        case SDEV_CANCEL:
        case SDEV_OFFLINE:
                break;
        default:
                return -EINVAL;
        }

Otherwise it looks fine to me.

Reply via email to