[GIT PULL] block bits

[GIT PULL] block bits

[Jens Axboe]

Hi Linus,

One critical bit in here, which fixes a reported oops from the iostat
patch. The others are either trivial/minor or documentation. Please
pull.

  git://git.kernel.dk/linux-2.6-block.git for-linus

Alberto Bertogli (2):
      Fix misleading comment in bio.h
      bio.h: If they MUST be inlined, then use __always_inline

Jens Axboe (3):
      block: fix oops in blk_queue_io_stat()
      block: fix inconsistent parenthesisation of QUEUE_FLAG_DEFAULT
      block: add text file detailing queue/ sysfs files

 Documentation/block/queue-sysfs.txt |   63 +++++++++++++++++++++++++++++++++++
 block/blk-core.c                    |    6 ++--
 block/blk.h                         |    8 ++++
 include/linux/bio.h                 |   10 +++--
 include/linux/blkdev.h              |    2 +-
 5 files changed, 81 insertions(+), 8 deletions(-)
 create mode 100644 Documentation/block/queue-sysfs.txt

diff --git a/Documentation/block/queue-sysfs.txt b/Documentation/block/queue-sysfs.txt
new file mode 100644
index 0000000..e164403
--- /dev/null
+++ b/Documentation/block/queue-sysfs.txt
@@ -0,0 +1,63 @@
+Queue sysfs files
+=================
+
+This text file will detail the queue files that are located in the sysfs tree
+for each block device. Note that stacked devices typically do not export
+any settings, since their queue merely functions are a remapping target.
+These files are the ones found in the /sys/block/xxx/queue/ directory.
+
+Files denoted with a RO postfix are readonly and the RW postfix means
+read-write.
+
+hw_sector_size (RO)
+-------------------
+This is the hardware sector size of the device, in bytes.
+
+max_hw_sectors_kb (RO)
+----------------------
+This is the maximum number of kilobytes supported in a single data transfer.
+
+max_sectors_kb (RW)
+-------------------
+This is the maximum number of kilobytes that the block layer will allow
+for a filesystem request. Must be smaller than or equal to the maximum
+size allowed by the hardware.
+
+nomerges (RW)
+-------------
+This enables the user to disable the lookup logic involved with IO merging
+requests in the block layer. Merging may still occur through a direct
+1-hit cache, since that comes for (almost) free. The IO scheduler will not
+waste cycles doing tree/hash lookups for merges if nomerges is 1. Defaults
+to 0, enabling all merges.
+
+nr_requests (RW)
+----------------
+This controls how many requests may be allocated in the block layer for
+read or write requests. Note that the total allocated number may be twice
+this amount, since it applies only to reads or writes (not the accumulated
+sum).
+
+read_ahead_kb (RW)
+------------------
+Maximum number of kilobytes to read-ahead for filesystems on this block
+device.
+
+rq_affinity (RW)
+----------------
+If this option is enabled, the block layer will migrate request completions
+to the CPU that originally submitted the request. For some workloads
+this provides a significant reduction in CPU cycles due to caching effects.
+
+scheduler (RW)
+--------------
+When read, this file will display the current and available IO schedulers
+for this block device. The currently active IO scheduler will be enclosed
+in [] brackets. Writing an IO scheduler name to this file will switch
+control of this block device to that new IO scheduler. Note that writing
+an IO scheduler name to this file will attempt to load that IO scheduler
+module, if it isn't already present in the system.
+
+
+
+Jens Axboe <jens.axboe@oracle.com>, February 2009
diff --git a/block/blk-core.c b/block/blk-core.c
index ca69f3d..29bcfac 100644
--- a/block/blk-core.c
+++ b/block/blk-core.c
@@ -69,7 +69,7 @@ static void drive_stat_acct(struct request *rq, int new_io)
 	int rw = rq_data_dir(rq);
 	int cpu;
 
-	if (!blk_fs_request(rq) || !disk || !blk_queue_io_stat(disk->queue))
+	if (!blk_fs_request(rq) || !disk || !blk_do_io_stat(disk->queue))
 		return;
 
 	cpu = part_stat_lock();
@@ -1667,7 +1667,7 @@ static void blk_account_io_completion(struct request *req, unsigned int bytes)
 {
 	struct gendisk *disk = req->rq_disk;
 
-	if (!disk || !blk_queue_io_stat(disk->queue))
+	if (!disk || !blk_do_io_stat(disk->queue))
 		return;
 
 	if (blk_fs_request(req)) {
@@ -1686,7 +1686,7 @@ static void blk_account_io_done(struct request *req)
 {
 	struct gendisk *disk = req->rq_disk;
 
-	if (!disk || !blk_queue_io_stat(disk->queue))
+	if (!disk || !blk_do_io_stat(disk->queue))
 		return;
 
 	/*
diff --git a/block/blk.h b/block/blk.h
index 6e1ed40..0dce92c 100644
--- a/block/blk.h
+++ b/block/blk.h
@@ -108,4 +108,12 @@ static inline int blk_cpu_to_group(int cpu)
 #endif
 }
 
+static inline int blk_do_io_stat(struct request_queue *q)
+{
+	if (q)
+		return blk_queue_io_stat(q);
+
+	return 0;
+}
+
 #endif
diff --git a/include/linux/bio.h b/include/linux/bio.h
index 0942765..2aa283a 100644
--- a/include/linux/bio.h
+++ b/include/linux/bio.h
@@ -451,12 +451,13 @@ extern struct biovec_slab bvec_slabs[BIOVEC_NR_POOLS] __read_mostly;
 
 #ifdef CONFIG_HIGHMEM
 /*
- * remember to add offset! and never ever reenable interrupts between a
- * bvec_kmap_irq and bvec_kunmap_irq!!
+ * remember never ever reenable interrupts between a bvec_kmap_irq and
+ * bvec_kunmap_irq!
  *
  * This function MUST be inlined - it plays with the CPU interrupt flags.
  */
-static inline char *bvec_kmap_irq(struct bio_vec *bvec, unsigned long *flags)
+static __always_inline char *bvec_kmap_irq(struct bio_vec *bvec,
+		unsigned long *flags)
 {
 	unsigned long addr;
 
@@ -472,7 +473,8 @@ static inline char *bvec_kmap_irq(struct bio_vec *bvec, unsigned long *flags)
 	return (char *) addr + bvec->bv_offset;
 }
 
-static inline void bvec_kunmap_irq(char *buffer, unsigned long *flags)
+static __always_inline void bvec_kunmap_irq(char *buffer,
+		unsigned long *flags)
 {
 	unsigned long ptr = (unsigned long) buffer & PAGE_MASK;
 
diff --git a/include/linux/blkdev.h b/include/linux/blkdev.h
index d08c4b8..dcaa0fd 100644
--- a/include/linux/blkdev.h
+++ b/include/linux/blkdev.h
@@ -455,7 +455,7 @@ struct request_queue
 
 #define QUEUE_FLAG_DEFAULT	((1 << QUEUE_FLAG_IO_STAT) |		\
 				 (1 << QUEUE_FLAG_CLUSTER) |		\
-				  1 << QUEUE_FLAG_STACKABLE)
+				 (1 << QUEUE_FLAG_STACKABLE))
 
 static inline int queue_is_locked(struct request_queue *q)
 {

-- 
Jens Axboe

Re: [GIT PULL] block bits

[Boaz Harrosh]

Jens Axboe wrote:
> Hi Linus,
> 
> One critical bit in here, which fixes a reported oops from the iostat
> patch. The others are either trivial/minor or documentation. Please
> pull.
> 
>   git://git.kernel.dk/linux-2.6-block.git for-linus
> 
> Alberto Bertogli (2):
>       Fix misleading comment in bio.h
>       bio.h: If they MUST be inlined, then use __always_inline
> 
> Jens Axboe (3):
>       block: fix oops in blk_queue_io_stat()
>       block: fix inconsistent parenthesisation of QUEUE_FLAG_DEFAULT
>       block: add text file detailing queue/ sysfs files
> 

Hi Jens.

The bsg sense handling bug fix, is I think, a security breach in that
Kernel copies dangling Kernel stack back to user-mode. It might even
need a stable release.

At the time I was thinking that since bsg is marked EXPERIMENTAL in
Kconfig then it must not be included in distro's Kernels by default,
but Benny noted that it is included in Fedora since FC8 or FC9.

The patch I sent still applies I think?

Thanks
Boaz

Re: [GIT PULL] block bits

[Jens Axboe]

On Mon, Feb 02 2009, Boaz Harrosh wrote:
> Jens Axboe wrote:
> > Hi Linus,
> > 
> > One critical bit in here, which fixes a reported oops from the iostat
> > patch. The others are either trivial/minor or documentation. Please
> > pull.
> > 
> >   git://git.kernel.dk/linux-2.6-block.git for-linus
> > 
> > Alberto Bertogli (2):
> >       Fix misleading comment in bio.h
> >       bio.h: If they MUST be inlined, then use __always_inline
> > 
> > Jens Axboe (3):
> >       block: fix oops in blk_queue_io_stat()
> >       block: fix inconsistent parenthesisation of QUEUE_FLAG_DEFAULT
> >       block: add text file detailing queue/ sysfs files
> > 
> 
> Hi Jens.
> 
> The bsg sense handling bug fix, is I think, a security breach in that
> Kernel copies dangling Kernel stack back to user-mode. It might even
> need a stable release.
> 
> At the time I was thinking that since bsg is marked EXPERIMENTAL in
> Kconfig then it must not be included in distro's Kernels by default,
> but Benny noted that it is included in Fedora since FC8 or FC9.
> 
> The patch I sent still applies I think?

I'm sure it does, I didn't know if Tomo had swoopt it up himself. I'll
add it to this branch.

-- 
Jens Axboe

[PATCH resend] bsg: Fix sense buffer bug in SG_IO

[Boaz Harrosh]

When submitting requests via SG_IO, which does a sync io, a
bsg_command is not allocated. So an in-Kernel sense_buffer was not
set. However when calling blk_execute_rq() with no sense buffer
one is provided from the stack. Now bsg at blk_complete_sgv4_hdr_rq()
would check if rq->sense_len and a sense was requested by sg_io_v4
the rq->sense was copy_user() back, but by now it is already mangled
stack memory.

I have fixed that by forcing a sense_buffer when calling bsg_map_hdr().
The bsg_command->sense is provided in the write/read path like before,
and on-the-stack buffer is provided when doing SG_IO.

I have also fixed a dprintk message to print rq->errors in hex because
of the scsi bit-field use of this member. For other block devices it
does not matter anyway.

Signed-off-by: Boaz Harrosh <bharrosh@panasas.com>
---
 block/bsg.c |   17 ++++++++++-------
 1 files changed, 10 insertions(+), 7 deletions(-)

diff --git a/block/bsg.c b/block/bsg.c
index d414bb5..0ce8806 100644
--- a/block/bsg.c
+++ b/block/bsg.c
@@ -244,7 +244,8 @@ bsg_validate_sgv4_hdr(struct request_queue *q, struct sg_io_v4 *hdr, int *rw)
  * map sg_io_v4 to a request.
  */
 static struct request *
-bsg_map_hdr(struct bsg_device *bd, struct sg_io_v4 *hdr, fmode_t has_write_perm)
+bsg_map_hdr(struct bsg_device *bd, struct sg_io_v4 *hdr, fmode_t has_write_perm,
+	    u8 *sense)
 {
 	struct request_queue *q = bd->queue;
 	struct request *rq, *next_rq = NULL;
@@ -306,6 +307,10 @@ bsg_map_hdr(struct bsg_device *bd, struct sg_io_v4 *hdr, fmode_t has_write_perm)
 		if (ret)
 			goto out;
 	}
+
+	rq->sense = sense;
+	rq->sense_len = 0;
+
 	return rq;
 out:
 	if (rq->cmd != rq->__cmd)
@@ -348,9 +353,6 @@ static void bsg_rq_end_io(struct request *rq, int uptodate)
 static void bsg_add_command(struct bsg_device *bd, struct request_queue *q,
 			    struct bsg_command *bc, struct request *rq)
 {
-	rq->sense = bc->sense;
-	rq->sense_len = 0;
-
 	/*
 	 * add bc command to busy queue and submit rq for io
 	 */
@@ -419,7 +421,7 @@ static int blk_complete_sgv4_hdr_rq(struct request *rq, struct sg_io_v4 *hdr,
 {
 	int ret = 0;
 
-	dprintk("rq %p bio %p %u\n", rq, bio, rq->errors);
+	dprintk("rq %p bio %p 0x%x\n", rq, bio, rq->errors);
 	/*
 	 * fill in all the output members
 	 */
@@ -635,7 +637,7 @@ static int __bsg_write(struct bsg_device *bd, const char __user *buf,
 		/*
 		 * get a request, fill in the blanks, and add to request queue
 		 */
-		rq = bsg_map_hdr(bd, &bc->hdr, has_write_perm);
+		rq = bsg_map_hdr(bd, &bc->hdr, has_write_perm, bc->sense);
 		if (IS_ERR(rq)) {
 			ret = PTR_ERR(rq);
 			rq = NULL;
@@ -922,11 +924,12 @@ static long bsg_ioctl(struct file *file, unsigned int cmd, unsigned long arg)
 		struct request *rq;
 		struct bio *bio, *bidi_bio = NULL;
 		struct sg_io_v4 hdr;
+		u8 sense[SCSI_SENSE_BUFFERSIZE];
 
 		if (copy_from_user(&hdr, uarg, sizeof(hdr)))
 			return -EFAULT;
 
-		rq = bsg_map_hdr(bd, &hdr, file->f_mode & FMODE_WRITE);
+		rq = bsg_map_hdr(bd, &hdr, file->f_mode & FMODE_WRITE, sense);
 		if (IS_ERR(rq))
 			return PTR_ERR(rq);
 
-- 
1.6.0.1

Re: [PATCH resend] bsg: Fix sense buffer bug in SG_IO

[Jens Axboe]

On Tue, Feb 03 2009, Boaz Harrosh wrote:
> 
> When submitting requests via SG_IO, which does a sync io, a
> bsg_command is not allocated. So an in-Kernel sense_buffer was not
> set. However when calling blk_execute_rq() with no sense buffer
> one is provided from the stack. Now bsg at blk_complete_sgv4_hdr_rq()
> would check if rq->sense_len and a sense was requested by sg_io_v4
> the rq->sense was copy_user() back, but by now it is already mangled
> stack memory.
> 
> I have fixed that by forcing a sense_buffer when calling bsg_map_hdr().
> The bsg_command->sense is provided in the write/read path like before,
> and on-the-stack buffer is provided when doing SG_IO.
> 
> I have also fixed a dprintk message to print rq->errors in hex because
> of the scsi bit-field use of this member. For other block devices it
> does not matter anyway.

Thanks for the resend Boaz, I'll make sure it gets upstream for 2.6.29.

-- 
Jens Axboe

Re: [PATCH resend] bsg: Fix sense buffer bug in SG_IO

[FUJITA Tomonori]

On Tue, 03 Feb 2009 08:34:07 +0200
Boaz Harrosh <bharrosh@panasas.com> wrote:

> 
> When submitting requests via SG_IO, which does a sync io, a
> bsg_command is not allocated. So an in-Kernel sense_buffer was not
> set. However when calling blk_execute_rq() with no sense buffer
> one is provided from the stack. Now bsg at blk_complete_sgv4_hdr_rq()
> would check if rq->sense_len and a sense was requested by sg_io_v4
> the rq->sense was copy_user() back, but by now it is already mangled
> stack memory.
> 
> I have fixed that by forcing a sense_buffer when calling bsg_map_hdr().
> The bsg_command->sense is provided in the write/read path like before,
> and on-the-stack buffer is provided when doing SG_IO.
> 
> I have also fixed a dprintk message to print rq->errors in hex because
> of the scsi bit-field use of this member. For other block devices it
> does not matter anyway.
> 
> Signed-off-by: Boaz Harrosh <bharrosh@panasas.com>
> ---
>  block/bsg.c |   17 ++++++++++-------
>  1 files changed, 10 insertions(+), 7 deletions(-)

Acked-by: FUJITA Tomonori <fujita.tomonori@lab.ntt.co.jp>


I think I did this in the previous submission though.

Re: [PATCH resend] bsg: Fix sense buffer bug in SG_IO

[Jens Axboe]

On Tue, Feb 03 2009, FUJITA Tomonori wrote:
> On Tue, 03 Feb 2009 08:34:07 +0200
> Boaz Harrosh <bharrosh@panasas.com> wrote:
> 
> > 
> > When submitting requests via SG_IO, which does a sync io, a
> > bsg_command is not allocated. So an in-Kernel sense_buffer was not
> > set. However when calling blk_execute_rq() with no sense buffer
> > one is provided from the stack. Now bsg at blk_complete_sgv4_hdr_rq()
> > would check if rq->sense_len and a sense was requested by sg_io_v4
> > the rq->sense was copy_user() back, but by now it is already mangled
> > stack memory.
> > 
> > I have fixed that by forcing a sense_buffer when calling bsg_map_hdr().
> > The bsg_command->sense is provided in the write/read path like before,
> > and on-the-stack buffer is provided when doing SG_IO.
> > 
> > I have also fixed a dprintk message to print rq->errors in hex because
> > of the scsi bit-field use of this member. For other block devices it
> > does not matter anyway.
> > 
> > Signed-off-by: Boaz Harrosh <bharrosh@panasas.com>
> > ---
> >  block/bsg.c |   17 ++++++++++-------
> >  1 files changed, 10 insertions(+), 7 deletions(-)
> 
> Acked-by: FUJITA Tomonori <fujita.tomonori@lab.ntt.co.jp>
> 
> 
> I think I did this in the previous submission though.

Yeah, I also added it.

-- 
Jens Axboe

Re: [osd-dev] Kernel crashes on login

[Boaz Harrosh]

Boaz Harrosh wrote:
> Fuerst Lars wrote:
>> Hello Boaz,
>>
>> I try to use your kernel initiator in combination with the OSC osd2
>> target. The target (FC 9, 2.6.26.5-45.fc9.x86_64) works fine with the
>> user space initiator from osc, but when i use your initiator (in tree
>> build 2.6.29-rc1-9) i get a failure after login:
>>
>> # iscsiadm -m node -p 141.11.96.100 -T render01crh --login
>> Logging in to [iface: default, target: render01crh, portal:
>> 141.11.96.100,3260]
>> iscsiadm: got read error (0/0), daemon died?
>>
>> iscsiadm: Could not login to [iface: default, target: render01crh,
>> portal: 141.11.96.100,3260]: 
>> iscsiadm: initiator reported error (18 - could not communicate to
>> iscsid)  
>>
>> kernel log says:
>>
>> [..]
>> Feb  5 14:29:16 render02crh kernel: osd:
>> OSD_ATTR_RI_VENDOR_IDENTIFICATION [<NULL>]
>>
>> Feb  5 14:29:16 render02crh kernel: osd:
>> OSD_ATTR_RI_PRODUCT_IDENTIFICATION [<NULL>]
>>
>> Feb  5 14:29:16 render02crh kernel: osd: OSD_ATTR_RI_PRODUCT_MODEL
>> [<NULL>]                                                             
>> Feb  5 14:29:16 render02crh kernel: BUG: unable to handle kernel NULL
>> pointer dereference at (null)                                     
>> Feb  5 14:29:16 render02crh kernel: IP: [<ffffffffa3114aa3>]
>> _osd_print_system_info+0x177/0x29b [libosd]
>>
>> Feb  5 14:29:16 render02crh kernel: PGD 6ad4d067 PUD 785d5067 PMD 0
>>
>> Feb  5 14:29:16 render02crh kernel: Oops: 0000 [#1] SMP
>>
>> Feb  5 14:29:16 render02crh kernel: last sysfs file:
>> /sys/devices/platform/host5/scsi_host/host5/scan
>> [..]
>>
>> Complete log and kernel config is attached. It seems to be a problem
>> during the osd auto detection. Do you have an idea?
>>
>> Thanks in advance
>>
>> Regards,
>>
>> Lars
> 
> Thanks Lars, It should not crash, I'll fix that ASAP
> 
> But please use the OSC's target from the open-osd.org
> git trees. Please follow the instructions in this page:
> http://www.open-osd.org/bin/view/Main/OscOsdProject
> Specifically the "How to get started" section.
> This will also automatically fetch the tgt module with all
> the right patches applied. So it should be easier to use.
> 
> The original version is missing some mandatory attributes
> and other incompatibilities with OSD2 been crafted on draft
> 01, when currently we're on draft 04d.
> 
> But sorry I should have checked that. Code should not crash
> ever.
> 
> I'll send a fix today.
> 
> Boaz
> 

Nice things about having some users is that they get to find
your bugs ;) . Thanks Lars

Some project History:

  At git://git.open-osd.org/osc-osd, our OSC's osd-target git tree,
there are a couple of fixes and enhancements over the latest source
code from OSC's internal SVN.

  Prior to that when we first started to use OSC's target we sent to
Pete and Anath fixes regarding none conforming behavior with regard
to attributes. The tar balls that where public on the net at the time
had these short comings. 

Lars are you using the old tar balls, I thought they where removed
from the net around the time Pete left OSC. (By accident)

The latest SVN code included our fixes, (and did not fully compile).
Are only publicly available on above git in the "osc" branch.
I have tested with that version and it does not crash so it might
be that you are using the old tar balls. In that case even with my
patch to fix the crash, attributes will not work at all and you will
not be able to work with open-osd initiator. :-(

It is strongly recommended that you upgrade to the open-osd version of
OSC's osd-target. (It also includes a matching OSC's osd-initiator)

I'm sending a patch as it's own email thread to James for inclusion
into 2.6.30.

Thank you for trying out and helping with the open-osd initiator.

Boaz