LKML Archive on lore.kernel.org
help / color / mirror / Atom feed
From: Sebastian Ott <sebott@linux.ibm.com>
To: Christoph Hellwig <hch@infradead.org>
Cc: Martin Schwidefsky <schwidefsky@de.ibm.com>,
	Linus Torvalds <torvalds@linux-foundation.org>,
	linux-kernel <linux-kernel@vger.kernel.org>,
	linux-s390 <linux-s390@vger.kernel.org>,
	Heiko Carstens <heiko.carstens@de.ibm.com>
Subject: Re: [GIT PULL] two more s390 bug fixes for 4.17
Date: Wed, 30 May 2018 14:59:58 +0200 (CEST)	[thread overview]
Message-ID: <alpine.LFD.2.21.1805301425580.1717@schleppi> (raw)
In-Reply-To: <20180530074130.GA6927@infradead.org>

On Wed, 30 May 2018, Christoph Hellwig 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.

Would you agree that this is an improvement over using completion_data
(at least now we don't mess with the block layers internal data)?

I already looked into what needs to be done to put struct dasd_ccw_req
behind the request and let it be managed by the block layer.

My concerns are lifetime and memory constraints (some of our HW interfaces
still use 31 bit pointers) but I think both of these are manageable with
the blk-mq interfaces.

But this would not be a small change and nothing for rc7. Would you be
OK with doing the small fix now and I'll work on the mentioned
improvement?

Regards,
Sebastian

  parent reply	other threads:[~2018-05-30 13:00 UTC|newest]

Thread overview: 7+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-05-30  5:59 Martin Schwidefsky
2018-05-30  7:41 ` Christoph Hellwig
2018-05-30  9:11   ` Martin Schwidefsky
2018-05-31 16:14     ` Christoph Hellwig
2018-06-04  8:20       ` Martin Schwidefsky
2018-05-30 12:59   ` Sebastian Ott [this message]
2018-06-08 17:40     ` Sebastian Ott

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=alpine.LFD.2.21.1805301425580.1717@schleppi \
    --to=sebott@linux.ibm.com \
    --cc=hch@infradead.org \
    --cc=heiko.carstens@de.ibm.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-s390@vger.kernel.org \
    --cc=schwidefsky@de.ibm.com \
    --cc=torvalds@linux-foundation.org \
    --subject='Re: [GIT PULL] two more s390 bug fixes for 4.17' \
    /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: 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).