[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <20180531161400.GB10203@infradead.org>
Date: Thu, 31 May 2018 09:14:00 -0700
From: Christoph Hellwig <hch@...radead.org>
To: Martin Schwidefsky <schwidefsky@...ibm.com>
Cc: Christoph Hellwig <hch@...radead.org>,
Linus Torvalds <torvalds@...ux-foundation.org>,
linux-kernel <linux-kernel@...r.kernel.org>,
linux-s390 <linux-s390@...r.kernel.org>,
Heiko Carstens <heiko.carstens@...ibm.com>
Subject: Re: [GIT PULL] two more s390 bug fixes for 4.17
On Wed, May 30, 2018 at 11:11:40AM +0200, Martin Schwidefsky wrote:
> On Wed, 30 May 2018 00:41:30 -0700
> Christoph Hellwig <hch@...radead.org> wrote:
>
> > > - req->completion_data = cqr;
> > > + *((struct dasd_ccw_req **) blk_mq_rq_to_pdu(req)) = cqr;
> > > +
> >
> > Please don't play such tricks. In general your driver structure
> > should have struct request embedded. If for some reason
> > struct dasd_ccw_req has a different life time please create a new
> > structure instead of these hacks.
>
> Why do you consider this to be a 'trick'? The blk_mq_rq_to_pdu is
> meant to be used to access a block of data that is is associated
> with a request, no? With the change we store a single value, the
> pointer to a struct dasd_ccw_req. The struct request comes first,
> later do_dasd_request creates the struct dasd_ccw_req with the
> CCW chain for the request. And for the blk timeout handler we need
> a way to find the dasd_ccw_req again.
We'll we try really hard to have a structure that we can use
container_of on. At least a minimal container with just the pointer
for a quick fix, but in general it seems like you should be able
to allocate the whole dasd_ccw_req with the request and just initialize
it later.
Powered by blists - more mailing lists