linux-kernel - Re: [PATCH v2 01/11] scsi: Convert struct Scsi_Host->cmd_serial_number to atomic

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:	Fri, 24 Sep 2010 13:41:20 -0700
From:	"Nicholas A. Bellinger" <nab@...ux-iscsi.org>
To:	Jens Axboe <axboe@...nel.dk>
Cc:	Mike Christie <michaelc@...wisc.edu>,
	Joe Eykholt <jeykholt@...co.com>,
	linux-scsi <linux-scsi@...r.kernel.org>,
	linux-kernel <linux-kernel@...r.kernel.org>,
	Vasu Dev <vasu.dev@...ux.intel.com>,
	Tim Chen <tim.c.chen@...ux.intel.com>,
	Andi Kleen <ak@...ux.intel.com>,
	Matthew Wilcox <willy@...ux.intel.com>,
	James Bottomley <James.Bottomley@...e.de>,
	James Smart <james.smart@...lex.com>,
	Andrew Vasquez <andrew.vasquez@...gic.com>,
	FUJITA Tomonori <fujita.tomonori@....ntt.co.jp>,
	Hannes Reinecke <hare@...e.de>, Christoph Hellwig <hch@....de>,
	Tejun Heo <tj@...nel.org>
Subject: Re: [PATCH v2 01/11] scsi: Convert struct 
	Scsi_Host->cmd_serial_number to atomic_t

On Fri, 2010-09-24 at 08:32 +0200, Jens Axboe wrote:
> On 2010-09-23 23:46, Nicholas A. Bellinger wrote:
> > On Sat, 2010-09-18 at 12:58 -0700, Nicholas A. Bellinger wrote:

<SNIP>

> > diff --git a/drivers/scsi/scsi_error.c b/drivers/scsi/scsi_error.c
> > index 1de30eb..f35c127 100644
> > --- a/drivers/scsi/scsi_error.c
> > +++ b/drivers/scsi/scsi_error.c
> > @@ -644,11 +644,7 @@ static int __scsi_try_to_abort_cmd(struct scsi_cmnd *scmd)
> >   */
> >  static int scsi_try_to_abort_cmd(struct scsi_cmnd *scmd)
> >  {
> > -       /*
> > -        * scsi_done was called just after the command timed out and before
> > -        * we had a chance to process it. (db)
> > -        */
> > -       if (scmd->serial_number == 0)
> > +       if (test_bit(REQ_ATOM_COMPLETE, &scmd->request->atomic_flags))
> >                 return SUCCESS;
> >         return __scsi_try_to_abort_cmd(scmd);
> >  }
> > 
> > and while building I noticed that the simple single enum
> > REQ_ATOM_COMPLETE=0 is
> 
> > currently located in block/blk.h and along with blk_mark_rq_complete()
> > and blk_clear_rq_complete() for setting this bit within struct
> > request->atomic_flags.
> > 
> > jens, hch, tejun, and co, do you guys have a preference how this
> > should be handled so that scsi_try_to_abort_cmd() can use proper
> > atomic struct request bits here and we can get rid of this (racy..?)
> > method of using struct scsi_cmnd->serial_number for anything wrt to
> > per struct scsi_cmnd context timeout handling.
> 
> Just add a
> 
> static inline int blk_test_rq_complete(struct request *rq)
> {
>         return test_bit(REQ_ATOM_COMPLETE, &rq->atomic_flags);
> }
> 
> in block/blk.h
> 
> I need this too for some lockless completion patches I am playing with.
> 

Hi Jens,

Perfect, thanks for the heads up on this wrapper.  I will go ahead and
include this into a RFCv4 series and update scsi_try_to_abort_cmd()
accordingly.

Best,

--nab


--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/