Alan Stern | 1 Jan 2010 03:46
Picon
Favicon

Re: [osd-dev] [PATCH] scsi_lib: Bug in completion of bidi commands

On Thu, 31 Dec 2009, Boaz Harrosh wrote:

> James hi.
> 
> What about this BUG. It affects anybody doing bidi commands. The possibilities
> are an sglist leak at best, and a crash at worse.
> 
> I understand this code needs cleanup, but first things first. Lets first fix the
> bug, which should also go to stable. Then the cleanup can go to next merge window.
> 
> BTW: Should I attempt a cleanup on current code, or should I wait for Alan's Patch
> to go in first?

What patch of mine are you referring to?  So far James has rejected all
the patches I have submitted recently.  I'm going to try again in the 
near future...

Alan Stern

--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majordomo <at> vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

bugzilla-daemon | 3 Jan 2010 02:42

[Bug 13783] udev causes high cpu usage

http://bugzilla.kernel.org/show_bug.cgi?id=13783

--- Comment #24 from jt512 <at> jt512.dyndns.org  2010-01-03 01:42:32 ---
Is anybody working on this bug?  It is being reported by numerous people in the
Archlinux forums.

--

-- 
Configure bugmail: http://bugzilla.kernel.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majordomo <at> vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Alan Stern | 3 Jan 2010 17:55
Picon
Favicon

Re: [osd-dev] [PATCH] scsi_lib: Bug in completion of bidi commands

On Sun, 3 Jan 2010, Boaz Harrosh wrote:

> > What patch of mine are you referring to?  So far James has rejected all
> > the patches I have submitted recently.  I'm going to try again in the 
> > near future...
> > 
> 
> OK, that's my answer, I didn't know.
> 
> Would you want that I attempt that collapsing of scsi_end_request() into scsi_io_completion
> and the cleanup that implies? (that's the patch I meant.)

Okay, I don't mind if you would like to rewrite that patch.  The 
version I wrote didn't just move code from one subroutine to another; 
it also made a few semantic changes (the retry counter and the "error" 
argument to blk_end_request()).  You'll probably want to break it 
up into a few patches, where the first simply moves the code around and 
the later ones do more significant things.

As I recall, the most recent version of that patch is here:

	http://marc.info/?l=linux-scsi&m=123991011815404&w=2

Alan Stern

--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majordomo <at> vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

(Continue reading)

Douglas Gilbert | 3 Jan 2010 19:51
Picon

[PATCH] skip sense logging for some ATA PASS-THROUGH cdbs

Further to the lsml thread titled:
"does scsi_io_completion need to dump sense data for ata pass through (ck_cond =
1) ?"

This is a patch to skip logging when the sense data is
associated with a SENSE_KEY of "RECOVERED_ERROR" and the
additional sense code is "ATA PASS-THROUGH INFORMATION
AVAILABLE". This only occurs with the SAT ATA PASS-THROUGH
commands when CK_COND=1 (in the cdb). It indicates that
the sense data contains ATA registers.

Smartmontools uses such commands on ATA disks connected via
SAT. Periodic checks such as those done by smartd cause
nuisance entries into logs that are:
    - neither errors nor warnings
    - pointless unless the cdb that caused them are also logged

The patch is against lk 2.6.32

Doug Gilbert

Attachment (scsi_lib2632smart1.patch): text/x-patch, 848 bytes
Boaz Harrosh | 4 Jan 2010 08:26
Favicon
Gravatar

Re: [osd-dev] [PATCH] scsi_lib: Bug in completion of bidi commands

On 01/03/2010 06:55 PM, Alan Stern wrote:
> On Sun, 3 Jan 2010, Boaz Harrosh wrote:
> 
>>> What patch of mine are you referring to?  So far James has rejected all
>>> the patches I have submitted recently.  I'm going to try again in the 
>>> near future...
>>>
>>
>> OK, that's my answer, I didn't know.
>>
>> Would you want that I attempt that collapsing of scsi_end_request() into scsi_io_completion
>> and the cleanup that implies? (that's the patch I meant.)
> 
> Okay, I don't mind if you would like to rewrite that patch.  The 
> version I wrote didn't just move code from one subroutine to another; 
> it also made a few semantic changes (the retry counter and the "error" 
> argument to blk_end_request()).  You'll probably want to break it 
> up into a few patches, where the first simply moves the code around and 
> the later ones do more significant things.
> 

Hi Alan, thanks

I'll only do the former and I'll let you continue with the later. .I.E the
code rearrangement and cleanup. Then perhaps it would be easier for you to
enhance the code with the retries and error returns. I do not have the setup
that can test / demonstrate those fixes, I'd rather you did them.

> As I recall, the most recent version of that patch is here:
> 
(Continue reading)

Bart Van Assche | 4 Jan 2010 12:37
Picon

Re: [PATCH] SCSI/libsrp: fix typo -- replace RDAM by RDMA

On Fri, Dec 4, 2009 at 8:38 PM, Bart Van Assche
<bart.vanassche <at> gmail.com> wrote:
>
> Fixed a typo in libsrp.c: replaced two occurrences of 'RDAM' by 'RDMA'.
>
> Signed-off-by: Bart Van Assche <bart.vanassche <at> gmail.com>
> Cc: James E.J. Bottomley <James.Bottomley <at> suse.de>
> Cc: FUJITA Tomonori <fujita.tomonori <at> lab.ntt.co.jp>
>
> diff --git a/drivers/scsi/libsrp.c b/drivers/scsi/libsrp.c
> index 9ad38e8..f5bfbc5 100644
> --- a/drivers/scsi/libsrp.c
> +++ b/drivers/scsi/libsrp.c
>  <at>  <at>  -1,5 +1,5  <at>  <at> 
>  /*
> - * SCSI RDAM Protocol lib functions
> + * SCSI RDMA Protocol lib functions
>  *
>  * Copyright (C) 2006 FUJITA Tomonori <tomof <at> acm.org>
>  *
>  <at>  <at>  -439,6 +439,6  <at>  <at>  int srp_cmd_queue(struct Scsi_Host *shost, struct srp_cmd *cmd, void *info,
>  }
>  EXPORT_SYMBOL_GPL(srp_cmd_queue);
>
> -MODULE_DESCRIPTION("SCSI RDAM Protocol lib functions");
> +MODULE_DESCRIPTION("SCSI RDMA Protocol lib functions");
>  MODULE_AUTHOR("FUJITA Tomonori");
>  MODULE_LICENSE("GPL");

Hello Fujita,
(Continue reading)

Boaz Harrosh | 4 Jan 2010 13:11
Favicon
Gravatar

[PATCHSET 0/3] little bit of love to scsi_io_completion


Three small incremental patches to clean up scsi_io_completion and friends.
There should be absolutely no functional change after this patchset, only
readability improvements.

This patchset should open up the stage for the fixes Alan needs and farther
enhancements.

(Mike, Hannes, I know you have pending work here, please point me to patches and
 I'll rebase them for you)

List of patches
[PATCH 1/3] scsi_lib: request_queue is only needed inside scsi_requeue_command
[PATCH 2/3] scsi_lib: Remove that __scsi_release_buffers contraption
[PATCH 3/3] scsi_lib: Collapse scsi_end_request into only user

Are on top of scsi-misc Please consider for inclusion
Boaz

--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majordomo <at> vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Boaz Harrosh | 4 Jan 2010 13:13
Favicon
Gravatar

[PATCH 1/3] scsi_lib: request_queue is only needed inside scsi_requeue_command


This is a pure cleanup. Just starting the engines for things
to come.

Signed-off-by: Boaz Harrosh <bharrosh <at> panasas.com>
---
 drivers/scsi/scsi_lib.c |   11 +++++------
 1 files changed, 5 insertions(+), 6 deletions(-)

diff --git a/drivers/scsi/scsi_lib.c b/drivers/scsi/scsi_lib.c
index c664242..05f988a 100644
--- a/drivers/scsi/scsi_lib.c
+++ b/drivers/scsi/scsi_lib.c
 <at>  <at>  -475,9 +475,10  <at>  <at>  static void scsi_run_queue(struct request_queue *q)
  *		sector.
  * Notes:	Upon return, cmd is a stale pointer.
  */
-static void scsi_requeue_command(struct request_queue *q, struct scsi_cmnd *cmd)
+static void scsi_requeue_command(struct scsi_cmnd *cmd)
 {
 	struct request *req = cmd->request;
+	struct request_queue *q = req->q;
 	unsigned long flags;

 	spin_lock_irqsave(q->queue_lock, flags);
 <at>  <at>  -538,7 +539,6  <at>  <at>  static void __scsi_release_buffers(struct scsi_cmnd *, int);
 static struct scsi_cmnd *scsi_end_request(struct scsi_cmnd *cmd, int error,
 					  int bytes, int requeue)
 {
-	struct request_queue *q = cmd->device->request_queue;
(Continue reading)

Boaz Harrosh | 4 Jan 2010 13:15
Favicon
Gravatar

[PATCH 2/3] scsi_lib: Remove that __scsi_release_buffers contraption


Remove "do_bidi_check" by checking and nullifying cmd->request
when blk_end_* indicates the request is no longer valid.

Signed-off-by: Boaz Harrosh <bharrosh <at> panasas.com>
---
 drivers/scsi/scsi_lib.c |   41 +++++++++++++++++------------------------
 1 files changed, 17 insertions(+), 24 deletions(-)

diff --git a/drivers/scsi/scsi_lib.c b/drivers/scsi/scsi_lib.c
index 05f988a..8d8b4eb 100644
--- a/drivers/scsi/scsi_lib.c
+++ b/drivers/scsi/scsi_lib.c
 <at>  <at>  -512,8 +512,6  <at>  <at>  void scsi_run_host_queues(struct Scsi_Host *shost)
 		scsi_run_queue(sdev->request_queue);
 }

-static void __scsi_release_buffers(struct scsi_cmnd *, int);
-
 /*
  * Function:    scsi_end_request()
  *
 <at>  <at>  -564,11 +562,12  <at>  <at>  static struct scsi_cmnd *scsi_end_request(struct scsi_cmnd *cmd, int error,
 		}
 	}

+	cmd->request = NULL;
 	/*
 	 * This will goose the queue request function at the end, so we don't
 	 * need to worry about launching another command.
(Continue reading)

Boaz Harrosh | 4 Jan 2010 13:17
Favicon
Gravatar

[PATCH 3/3] scsi_lib: Collapse scsi_end_request into only user


Embedding scsi_end_request() into scsi_io_completion actually simplifies the
code and makes it clearer what's going on.

There is absolutely no functional and/or side effects changes after this patch.

Patch was inspired by Alan Stern.

CC: Alan Stern <stern <at> rowland.harvard.edu>
Signed-off-by: Boaz Harrosh <bharrosh <at> panasas.com>
---
 drivers/scsi/scsi_lib.c |   90 +++++++++++-----------------------------------
 1 files changed, 22 insertions(+), 68 deletions(-)

diff --git a/drivers/scsi/scsi_lib.c b/drivers/scsi/scsi_lib.c
index 8d8b4eb..326b228 100644
--- a/drivers/scsi/scsi_lib.c
+++ b/drivers/scsi/scsi_lib.c
 <at>  <at>  -512,66 +512,6  <at>  <at>  void scsi_run_host_queues(struct Scsi_Host *shost)
 		scsi_run_queue(sdev->request_queue);
 }

-/*
- * Function:    scsi_end_request()
- *
- * Purpose:     Post-processing of completed commands (usually invoked at end
- *		of upper level post-processing and scsi_io_completion).
- *
- * Arguments:   cmd	 - command that is complete.
- *              error    - 0 if I/O indicates success, < 0 for I/O error.
(Continue reading)


Gmane