public inbox for linux-scsi@vger.kernel.org
 help / color / mirror / Atom feed
From: Vladislav Bolkhovitin <vst@vlnb.net>
To: FUJITA Tomonori <fujita.tomonori@lab.ntt.co.jp>
Cc: Kai.Makisara@kolumbus.fi, James.Bottomley@HansenPartnership.com,
	linux-scsi@vger.kernel.org
Subject: Re: [PATCH 00/11] st: remove scsi_execute_async usage (the first half)
Date: Mon, 01 Dec 2008 18:19:45 +0300	[thread overview]
Message-ID: <49340091.4070507@vlnb.net> (raw)
In-Reply-To: <1228032485-10328-1-git-send-email-fujita.tomonori@lab.ntt.co.jp>


FUJITA Tomonori wrote:
> This patchset removes the majority of scsi_execute_async in st
> driver. IOW, this converts st driver to use the block layer functions.
> 
> We are in the process of removing scsi_execute_async and
> scsi_req_map_sg. scsi_execute_async in sg driver were removed in
> 2.6.27. Now only st and osst drivers use scsi_execute_async().
> 
> st driver uses scsi_execute_async for two purposes, performing sort
> management SCSI commands internally and data transfer between user and
> kernel space (st_read and st_write). This patchset converts the
> former.
> 
> The former performs SCSI commands synchronously. It uses a liner
> in-kernel buffer (not scatter gather) for data transfer. To perform
> such commands, other upper layer drivers (e.g. sd) use
> scsi_execute_req (internally uses blk_rq_map_kern and and
> blk_execute_rq). scsi_execute_req is not fully fit for st so I adds a
> helper function similar to scsi_execute_req and replace
> scsi_execute_async in st driver (I might modify scsi_execute_req for
> st and remove the helper function later).
> 
> I'm still working on converting scsi_execute_async for data transfer
> between user and kernel space but I want to get this first half be
> merged.

May I ask a possibly stupid question? Is amount of overall code in Linux 
SCSI subsystem after your conversion is fully done and 
scsi_execute_async() with scsi_req_map_sg() completely removed going to 
get smaller or bigger?

 From diffstats of your patches seems it's is going to get considerably 
bigger (171 insertion vs 61 deletion). And this is only in one user of 
scsi_execute_async() of 4. What's the point then in scsi_execute_async() 
removal if the resulting code is going to get bigger, not smaller? And, 
frankly, it doesn't look as it's going to get clearer too..

P.S. Scsi_execute_async() is just ~50 lines long, scsi_req_map_sg() is 
just ~80 lines long and both look like nice helper functions, which are 
not worse than st_scsi_kern_execute().

Regards,
Vlad


  parent reply	other threads:[~2008-12-01 15:19 UTC|newest]

Thread overview: 26+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2008-11-30  8:07 [PATCH 00/11] st: remove scsi_execute_async usage (the first half) FUJITA Tomonori
2008-11-30  8:07 ` [PATCH 01/11] st: move st_request initialization to st_allocate_request form st_do_scsi FUJITA Tomonori
2008-11-30  8:07   ` [PATCH 02/11] st: add st_scsi_kern_execute helper function FUJITA Tomonori
2008-11-30  8:07     ` [PATCH 03/11] st: convert test_ready to use st_scsi_kern_execute FUJITA Tomonori
2008-11-30  8:07       ` [PATCH 04/11] st: convert set_location " FUJITA Tomonori
2008-11-30  8:07         ` [PATCH 05/11] st: convert do_load_unload " FUJITA Tomonori
2008-11-30  8:08           ` [PATCH 06/11] st: convert cross_eof " FUJITA Tomonori
2008-11-30  8:08             ` [PATCH 07/11] st: convert st_flush " FUJITA Tomonori
2008-11-30  8:08               ` [PATCH 08/11] st: convert check_tape " FUJITA Tomonori
2008-11-30  8:08                 ` [PATCH 09/11] st: convert read_mode_page " FUJITA Tomonori
2008-11-30  8:08                   ` [PATCH 10/11] st: convert write_mode_page " FUJITA Tomonori
2008-11-30  8:08                     ` [PATCH 11/11] st: convert get_location " FUJITA Tomonori
2008-11-30 12:12                   ` [PATCH 09/11] st: convert read_mode_page " Boaz Harrosh
2008-12-01  8:21                     ` FUJITA Tomonori
2008-11-30 12:10     ` [PATCH 02/11] st: add st_scsi_kern_execute helper function Boaz Harrosh
2008-12-01  8:15       ` FUJITA Tomonori
2008-12-01 15:19 ` Vladislav Bolkhovitin [this message]
2008-12-01 15:36   ` [PATCH 00/11] st: remove scsi_execute_async usage (the first half) Boaz Harrosh
2008-12-01 18:17     ` Vladislav Bolkhovitin
2008-12-02  7:52       ` Boaz Harrosh
2008-12-02 19:08 ` Kai Makisara
2008-12-03  0:27   ` FUJITA Tomonori
2008-12-03 19:40     ` Kai Makisara
2008-12-04  5:53       ` FUJITA Tomonori
2008-12-04 20:23         ` Kai Makisara
2008-12-05  6:25           ` FUJITA Tomonori

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=49340091.4070507@vlnb.net \
    --to=vst@vlnb.net \
    --cc=James.Bottomley@HansenPartnership.com \
    --cc=Kai.Makisara@kolumbus.fi \
    --cc=fujita.tomonori@lab.ntt.co.jp \
    --cc=linux-scsi@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: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox