Patchwork [RFC] The future of iscsitarget

login
register
mail settings
Submitter Stefan Bader
Date May 19, 2011, 2:43 p.m.
Message ID <4DD52CA7.9070100@canonical.com>
Download mbox | patch
Permalink /patch/96394/
State New
Headers show

Comments

Stefan Bader - May 19, 2011, 2:43 p.m.
We currently carry iscsitarget as one of our ubuntu specific modules. Due to the
compile breaking for Oneiric, I was looking into this. I found that, at least
starting with Maverick, there is a source package iscsitarget which also
produces a dkms module. The code of both is nearly identical, but have already
diverged due to fixups done to the kernel side while the dkms version just
stayed at the last upstream version.

First question would be whether we really need iscsitarget at all as there is an
in kernel scsi target framework[1]. The answer may be yes, because people
(still) use it.

But then it is not very efficient to provide the kernel module in the kernel
itself and a dkms package. I would tend to say it should be removed from the
kernel tree and only be provided by dkms as then all the code from the
iscsitarget project is in one package, the module should not be important during
installation as it only allows to use the host as a target and when fixing the
dkms build in the iscsitarget package, the changes maybe flow back via debian to
the original project (though it looked a bit like there has been no
activity/changes since end of last year).

What would be other peoples thoughts on this?

-Stefan

[1] http://lwn.net/Articles/424004/
Leann Ogasawara - May 19, 2011, 4:26 p.m.
On Thu, 2011-05-19 at 16:43 +0200, Stefan Bader wrote:
> We currently carry iscsitarget as one of our ubuntu specific modules. Due to the
> compile breaking for Oneiric, I was looking into this. I found that, at least
> starting with Maverick, there is a source package iscsitarget which also
> produces a dkms module. The code of both is nearly identical, but have already
> diverged due to fixups done to the kernel side while the dkms version just
> stayed at the last upstream version.
> 
> First question would be whether we really need iscsitarget at all as there is an
> in kernel scsi target framework[1]. The answer may be yes, because people
> (still) use it.
> 
> But then it is not very efficient to provide the kernel module in the kernel
> itself and a dkms package. I would tend to say it should be removed from the
> kernel tree and only be provided by dkms as then all the code from the
> iscsitarget project is in one package, the module should not be important during
> installation as it only allows to use the host as a target and when fixing the
> dkms build in the iscsitarget package, the changes maybe flow back via debian to
> the original project (though it looked a bit like there has been no
> activity/changes since end of last year).
> 
> What would be other peoples thoughts on this?

I'd be in favor of the DKMS package thus allowing us to drop it from the
Ubuntu kernel.  This has been our suggested policy for out of tree
drivers we've carried in the past.

Thanks,
Leann

> [1] http://lwn.net/Articles/424004/
Tim Gardner - May 25, 2011, 2:34 p.m.
On 05/19/2011 10:26 AM, Leann Ogasawara wrote:
> On Thu, 2011-05-19 at 16:43 +0200, Stefan Bader wrote:
>> We currently carry iscsitarget as one of our ubuntu specific modules. Due to the
>> compile breaking for Oneiric, I was looking into this. I found that, at least
>> starting with Maverick, there is a source package iscsitarget which also
>> produces a dkms module. The code of both is nearly identical, but have already
>> diverged due to fixups done to the kernel side while the dkms version just
>> stayed at the last upstream version.
>>
>> First question would be whether we really need iscsitarget at all as there is an
>> in kernel scsi target framework[1]. The answer may be yes, because people
>> (still) use it.
>>
>> But then it is not very efficient to provide the kernel module in the kernel
>> itself and a dkms package. I would tend to say it should be removed from the
>> kernel tree and only be provided by dkms as then all the code from the
>> iscsitarget project is in one package, the module should not be important during
>> installation as it only allows to use the host as a target and when fixing the
>> dkms build in the iscsitarget package, the changes maybe flow back via debian to
>> the original project (though it looked a bit like there has been no
>> activity/changes since end of last year).
>>
>> What would be other peoples thoughts on this?
>
> I'd be in favor of the DKMS package thus allowing us to drop it from the
> Ubuntu kernel.  This has been our suggested policy for out of tree
> drivers we've carried in the past.
>
> Thanks,
> Leann
>
>> [1] http://lwn.net/Articles/424004/
>
>
>

I'm good with that, especially since the in-kernel SCSI target support 
is in a state of flux.

rtg

Patch

From 5cf7286560ac39367f99319694dda889047208ca Mon Sep 17 00:00:00 2001
From: Stefan Bader <stefan.bader@canonical.com>
Date: Tue, 17 May 2011 17:45:00 +0200
Subject: [PATCH] UBUNTU: iscsitarget: Convert to new plugging method

The block device plugging used to be per-queue and implicitly
done when requests where added to an empty queue. Now callers
have to plug and unplug per-task.

This fixes a FTBS with 2.6.39 kernel code.

Signed-off-by: Stefan Bader <stefan.bader@canonical.com>
---
 debian.master/config/config.common.ubuntu |    2 +-
 ubuntu/iscsitarget/block-io.c             |    6 ++++--
 2 files changed, 5 insertions(+), 3 deletions(-)

diff --git a/debian.master/config/config.common.ubuntu b/debian.master/config/config.common.ubuntu
index cc4c4fe..12ddcdd 100644
--- a/debian.master/config/config.common.ubuntu
+++ b/debian.master/config/config.common.ubuntu
@@ -4096,7 +4096,7 @@  CONFIG_SCSI_IN2000=m
 CONFIG_SCSI_INIA100=m
 CONFIG_SCSI_INITIO=m
 CONFIG_SCSI_IPS=m
-# CONFIG_SCSI_ISCSITARGET is not set
+CONFIG_SCSI_ISCSITARGET=m
 CONFIG_SCSI_ISCSI_ATTRS=m
 # CONFIG_SCSI_IZIP_EPP16 is not set
 # CONFIG_SCSI_IZIP_SLOW_CTR is not set
diff --git a/ubuntu/iscsitarget/block-io.c b/ubuntu/iscsitarget/block-io.c
index 3e7de38..e3c9e19 100644
--- a/ubuntu/iscsitarget/block-io.c
+++ b/ubuntu/iscsitarget/block-io.c
@@ -56,6 +56,7 @@  blockio_make_request(struct iet_volume *volume, struct tio *tio, int rw)
 	struct request_queue *bdev_q = bdev_get_queue(bio_data->bdev);
 	struct tio_work *tio_work;
 	struct bio *tio_bio = NULL, *bio = NULL, *biotail = NULL;
+	struct blk_plug plug;
 
 	u32 offset = tio->offset;
 	u32 size = tio->size;
@@ -78,6 +79,8 @@  blockio_make_request(struct iet_volume *volume, struct tio *tio, int rw)
 	atomic_set(&tio_work->bios_remaining, 0);
 	init_completion(&tio_work->tio_complete);
 
+	blk_start_plug(&plug);
+
 	/* Main processing loop, allocate and fill all bios */
 	while (tio_index < tio->pg_cnt) {
 		bio = bio_alloc(GFP_KERNEL, min(max_pages, BIO_MAX_PAGES));
@@ -127,8 +130,7 @@  blockio_make_request(struct iet_volume *volume, struct tio *tio, int rw)
 		submit_bio(rw, bio);
 	}
 
-	if (bdev_q && bdev_q->unplug_fn)
-		bdev_q->unplug_fn(bdev_q);
+	blk_finish_plug(&plug);
 
 	wait_for_completion(&tio_work->tio_complete);
 
-- 
1.7.0.4