From: Mike Christie <michael.christie@oracle.com> To: Bodo Stroesser <bstroesser@ts.fujitsu.com>, "Martin K. Petersen" <martin.petersen@oracle.com>, linux-scsi@vger.kernel.org, target-devel@vger.kernel.org Subject: Re: [PATCH v2 8/8] scsi: target: tcmu: Make TMR notification optional Date: Sun, 26 Jul 2020 00:06:14 +0000 [thread overview] Message-ID: <454b34ff-47c6-f54d-7b52-642b08f8cbd1@oracle.com> (raw) In-Reply-To: <20200717161212.10731-9-bstroesser@ts.fujitsu.com> On 7/17/20 11:12 AM, Bodo Stroesser wrote: > Add "tmr_notification" configFS attribute to tcmu devices. > If default value 0 of the attribute is used, tcmu only > removes aborted commands from qfull_queue. > If user changes tmr_notification to 1, additionally > TMR notifications will be written to the cmd ring. > > Signed-off-by: Bodo Stroesser <bstroesser@ts.fujitsu.com> > --- > drivers/target/target_core_user.c | 39 +++++++++++++++++++++++++++++++++++++++ > 1 file changed, 39 insertions(+) > > diff --git a/drivers/target/target_core_user.c b/drivers/target/target_core_user.c > index 92b7a2e84e64..95e66b707373 100644 > --- a/drivers/target/target_core_user.c > +++ b/drivers/target/target_core_user.c > @@ -118,6 +118,7 @@ struct tcmu_dev { > #define TCMU_DEV_BIT_OPEN 0 > #define TCMU_DEV_BIT_BROKEN 1 > #define TCMU_DEV_BIT_BLOCKED 2 > +#define TCMU_DEV_BIT_TMR_NOTIFY 3 > unsigned long flags; > > struct uio_info uio_info; > @@ -1260,6 +1261,9 @@ tcmu_tmr_notify(struct se_device *se_dev, enum tcm_tmreq_table tmf, > if (unqueued) > tcmu_set_next_deadline(&dev->qfull_queue, &dev->qfull_timer); > > + if (!test_bit(TCMU_DEV_BIT_TMR_NOTIFY, &dev->flags)) > + goto unlock; > + > pr_debug("TMR event %d on dev %s, aborted cmds %d, afflicted cmd_ids %d\n", > tcmu_tmr_type(tmf), dev->name, i, cmd_cnt); > > @@ -2706,6 +2710,40 @@ static ssize_t tcmu_emulate_write_cache_store(struct config_item *item, > } > CONFIGFS_ATTR(tcmu_, emulate_write_cache); > > +static ssize_t tcmu_tmr_notification_show(struct config_item *item, > + char *page) Sorry about this. Just some nits. The spacing above got messed up a little. I think you only need 2 spaces, but it looks like we got some extras. > +{ > + struct se_dev_attrib *da = container_of(to_config_group(item), > + struct se_dev_attrib, da_group); > + struct tcmu_dev *dev = TCMU_DEV(da->da_dev); Could you use udev or tcmu_dev for the name. Sorry for being a broken record on this one. We use dev or se_dev for the se_device struct already and it throws me off when scanning the code. I think patch 5 and 7 need the same fix up.
WARNING: multiple messages have this Message-ID (diff)
From: Mike Christie <michael.christie@oracle.com> To: Bodo Stroesser <bstroesser@ts.fujitsu.com>, "Martin K. Petersen" <martin.petersen@oracle.com>, linux-scsi@vger.kernel.org, target-devel@vger.kernel.org Subject: Re: [PATCH v2 8/8] scsi: target: tcmu: Make TMR notification optional Date: Sat, 25 Jul 2020 19:06:14 -0500 [thread overview] Message-ID: <454b34ff-47c6-f54d-7b52-642b08f8cbd1@oracle.com> (raw) In-Reply-To: <20200717161212.10731-9-bstroesser@ts.fujitsu.com> On 7/17/20 11:12 AM, Bodo Stroesser wrote: > Add "tmr_notification" configFS attribute to tcmu devices. > If default value 0 of the attribute is used, tcmu only > removes aborted commands from qfull_queue. > If user changes tmr_notification to 1, additionally > TMR notifications will be written to the cmd ring. > > Signed-off-by: Bodo Stroesser <bstroesser@ts.fujitsu.com> > --- > drivers/target/target_core_user.c | 39 +++++++++++++++++++++++++++++++++++++++ > 1 file changed, 39 insertions(+) > > diff --git a/drivers/target/target_core_user.c b/drivers/target/target_core_user.c > index 92b7a2e84e64..95e66b707373 100644 > --- a/drivers/target/target_core_user.c > +++ b/drivers/target/target_core_user.c > @@ -118,6 +118,7 @@ struct tcmu_dev { > #define TCMU_DEV_BIT_OPEN 0 > #define TCMU_DEV_BIT_BROKEN 1 > #define TCMU_DEV_BIT_BLOCKED 2 > +#define TCMU_DEV_BIT_TMR_NOTIFY 3 > unsigned long flags; > > struct uio_info uio_info; > @@ -1260,6 +1261,9 @@ tcmu_tmr_notify(struct se_device *se_dev, enum tcm_tmreq_table tmf, > if (unqueued) > tcmu_set_next_deadline(&dev->qfull_queue, &dev->qfull_timer); > > + if (!test_bit(TCMU_DEV_BIT_TMR_NOTIFY, &dev->flags)) > + goto unlock; > + > pr_debug("TMR event %d on dev %s, aborted cmds %d, afflicted cmd_ids %d\n", > tcmu_tmr_type(tmf), dev->name, i, cmd_cnt); > > @@ -2706,6 +2710,40 @@ static ssize_t tcmu_emulate_write_cache_store(struct config_item *item, > } > CONFIGFS_ATTR(tcmu_, emulate_write_cache); > > +static ssize_t tcmu_tmr_notification_show(struct config_item *item, > + char *page) Sorry about this. Just some nits. The spacing above got messed up a little. I think you only need 2 spaces, but it looks like we got some extras. > +{ > + struct se_dev_attrib *da = container_of(to_config_group(item), > + struct se_dev_attrib, da_group); > + struct tcmu_dev *dev = TCMU_DEV(da->da_dev); Could you use udev or tcmu_dev for the name. Sorry for being a broken record on this one. We use dev or se_dev for the se_device struct already and it throws me off when scanning the code. I think patch 5 and 7 need the same fix up.
next prev parent reply other threads:[~2020-07-26 0:06 UTC|newest] Thread overview: 26+ messages / expand[flat|nested] mbox.gz Atom feed top 2020-07-17 16:12 [PATCH v2 0/8] scsi: target: tcmu: Add TMR notification for tcmu Bodo Stroesser 2020-07-17 16:12 ` Bodo Stroesser 2020-07-17 16:12 ` [PATCH v2 1/8] scsi: target: Modify core_tmr_abort_task() Bodo Stroesser 2020-07-17 16:12 ` Bodo Stroesser 2020-07-26 0:03 ` Mike Christie 2020-07-26 0:03 ` Mike Christie 2020-07-26 11:35 ` Bodo Stroesser 2020-07-26 11:35 ` Bodo Stroesser 2020-07-17 16:12 ` [PATCH v2 2/8] scsi: target: Add tmr_notify backend function Bodo Stroesser 2020-07-17 16:12 ` Bodo Stroesser 2020-07-17 16:12 ` [PATCH v2 3/8] scsi: target: tcmu: Use priv pointer in se_cmd Bodo Stroesser 2020-07-17 16:12 ` Bodo Stroesser 2020-07-17 16:12 ` [PATCH v2 4/8] scsi: target: tcmu: Do not queue aborted commands Bodo Stroesser 2020-07-17 16:12 ` Bodo Stroesser 2020-07-17 16:12 ` [PATCH v2 5/8] scsi: target: tcmu: Factor out new helper ring_insert_padding Bodo Stroesser 2020-07-17 16:12 ` Bodo Stroesser 2020-07-17 16:12 ` [PATCH v2 6/8] scsi: target: tcmu: Fix and simplify timeout handling Bodo Stroesser 2020-07-17 16:12 ` Bodo Stroesser 2020-07-17 16:12 ` [PATCH v2 7/8] scsi: target: tcmu: Implement tmr_notify callback Bodo Stroesser 2020-07-17 16:12 ` Bodo Stroesser 2020-07-17 16:12 ` [PATCH v2 8/8] scsi: target: tcmu: Make TMR notification optional Bodo Stroesser 2020-07-17 16:12 ` Bodo Stroesser 2020-07-26 0:06 ` Mike Christie [this message] 2020-07-26 0:06 ` Mike Christie 2020-07-26 11:47 ` Bodo Stroesser 2020-07-26 11:47 ` Bodo Stroesser
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: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --in-reply-to=454b34ff-47c6-f54d-7b52-642b08f8cbd1@oracle.com \ --to=michael.christie@oracle.com \ --cc=bstroesser@ts.fujitsu.com \ --cc=linux-scsi@vger.kernel.org \ --cc=martin.petersen@oracle.com \ --cc=target-devel@vger.kernel.org \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: linkBe sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.