Re: [PATCH 9/12] repost: cciss: add busy_configuring flag
On Fri, Jan 19 2007, dann frazier wrote: > On Wed, Dec 13, 2006 at 01:52:36PM +0100, Jens Axboe wrote: > > On Tue, Dec 12 2006, Mike Miller (OS Dev) wrote: > > > On Mon, Nov 06, 2006 at 09:32:00PM +0100, Jens Axboe wrote: > > > > On Mon, Nov 06 2006, Mike Miller (OS Dev) wrote: > > > > > PATCH 9 of 12 > > > > > > > > > > This patch adds a check for busy_configuring to prevent starting a > > > > > queue > > > > > on a drive that may be in the midst of updating, configuring, > > > > > deleting, etc. > > > > > > > > > > This had a test for if the queue was stopped or plugged but that > > > > > seemed > > > > > to cause issues. > > > > > Please consider this for inclusion. > > > > > > > > > > Thanks, > > > > > mikem > > > > > > > > > > Signed-off-by: Mike Miller <[EMAIL PROTECTED]> > > > > > > > > > > > > > > > > > > > > --- > > > > > > > > > > drivers/block/cciss.c |5 - > > > > > 1 files changed, 4 insertions(+), 1 deletion(-) > > > > > > > > > > diff -puN drivers/block/cciss.c~cciss_busy_conf_for_lx2619-rc4 > > > > > drivers/block/cciss.c > > > > > --- linux-2.6/drivers/block/cciss.c~cciss_busy_conf_for_lx2619-rc4 > > > > > 2006-11-06 13:27:53.0 -0600 > > > > > +++ linux-2.6-root/drivers/block/cciss.c 2006-11-06 > > > > > 13:27:53.0 -0600 > > > > > @@ -1190,8 +1190,11 @@ static void cciss_check_queues(ctlr_info > > > > > /* make sure the disk has been added and the drive is > > > > > real > > > > >* because this can be called from the middle of > > > > > init_one. > > > > >*/ > > > > > - if (!(h->drv[curr_queue].queue) || > > > > > !(h->drv[curr_queue].heads)) > > > > > + if (!(h->drv[curr_queue].queue) || > > > > > + !(h->drv[curr_queue].heads) || > > > > > + h->drv[curr_queue].busy_configuring) > > > > > continue; > > > > > + > > > > > blk_start_queue(h->gendisk[curr_queue]->queue); > > > > > > > > This is racy, because you don't start the queue when you unset > > > > ->busy_configuring later on. For this to be safe, you need to call > > > > blk_start_queue() when you set ->busy_configuring to 0. > > > > > > Jens, please see Chase's reply to your concerns: > > > > busy_configuring - I do not think this is racy. This > > > > flag is used only when we are removing/deleting a disk. In > > > > this case the queue is cleaned up and the disk is deleted. > > > > If we are doing that then there is no queue to start later. > > > > The check of this flag in the interrupt handler is to prevent > > > > us from trying to start a queue that is in the middle of > > > > being deleted. This flag could be called busy_deleting. > > > > Ok, no worries then if it's simply a going away flag. I wonder if it's > > needed at all, but it certainly doesn't hurt. > > hey Jens, > Just a poke since I haven't seen this change go into your block > tree. Is it still in-plan? I had it stashed for a 2.6.21 merge, it'll go in by then. -- Jens Axboe - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 9/12] repost: cciss: add busy_configuring flag
On Fri, Jan 19 2007, dann frazier wrote: On Wed, Dec 13, 2006 at 01:52:36PM +0100, Jens Axboe wrote: On Tue, Dec 12 2006, Mike Miller (OS Dev) wrote: On Mon, Nov 06, 2006 at 09:32:00PM +0100, Jens Axboe wrote: On Mon, Nov 06 2006, Mike Miller (OS Dev) wrote: PATCH 9 of 12 This patch adds a check for busy_configuring to prevent starting a queue on a drive that may be in the midst of updating, configuring, deleting, etc. This had a test for if the queue was stopped or plugged but that seemed to cause issues. Please consider this for inclusion. Thanks, mikem Signed-off-by: Mike Miller [EMAIL PROTECTED] --- drivers/block/cciss.c |5 - 1 files changed, 4 insertions(+), 1 deletion(-) diff -puN drivers/block/cciss.c~cciss_busy_conf_for_lx2619-rc4 drivers/block/cciss.c --- linux-2.6/drivers/block/cciss.c~cciss_busy_conf_for_lx2619-rc4 2006-11-06 13:27:53.0 -0600 +++ linux-2.6-root/drivers/block/cciss.c 2006-11-06 13:27:53.0 -0600 @@ -1190,8 +1190,11 @@ static void cciss_check_queues(ctlr_info /* make sure the disk has been added and the drive is real * because this can be called from the middle of init_one. */ - if (!(h-drv[curr_queue].queue) || !(h-drv[curr_queue].heads)) + if (!(h-drv[curr_queue].queue) || + !(h-drv[curr_queue].heads) || + h-drv[curr_queue].busy_configuring) continue; + blk_start_queue(h-gendisk[curr_queue]-queue); This is racy, because you don't start the queue when you unset -busy_configuring later on. For this to be safe, you need to call blk_start_queue() when you set -busy_configuring to 0. Jens, please see Chase's reply to your concerns: busy_configuring - I do not think this is racy. This flag is used only when we are removing/deleting a disk. In this case the queue is cleaned up and the disk is deleted. If we are doing that then there is no queue to start later. The check of this flag in the interrupt handler is to prevent us from trying to start a queue that is in the middle of being deleted. This flag could be called busy_deleting. Ok, no worries then if it's simply a going away flag. I wonder if it's needed at all, but it certainly doesn't hurt. hey Jens, Just a poke since I haven't seen this change go into your block tree. Is it still in-plan? I had it stashed for a 2.6.21 merge, it'll go in by then. -- Jens Axboe - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 9/12] repost: cciss: add busy_configuring flag
On Wed, Dec 13, 2006 at 01:52:36PM +0100, Jens Axboe wrote: > On Tue, Dec 12 2006, Mike Miller (OS Dev) wrote: > > On Mon, Nov 06, 2006 at 09:32:00PM +0100, Jens Axboe wrote: > > > On Mon, Nov 06 2006, Mike Miller (OS Dev) wrote: > > > > PATCH 9 of 12 > > > > > > > > This patch adds a check for busy_configuring to prevent starting a queue > > > > on a drive that may be in the midst of updating, configuring, deleting, > > > > etc. > > > > > > > > This had a test for if the queue was stopped or plugged but that seemed > > > > to cause issues. > > > > Please consider this for inclusion. > > > > > > > > Thanks, > > > > mikem > > > > > > > > Signed-off-by: Mike Miller <[EMAIL PROTECTED]> > > > > > > > > > > > > > > > > --- > > > > > > > > drivers/block/cciss.c |5 - > > > > 1 files changed, 4 insertions(+), 1 deletion(-) > > > > > > > > diff -puN drivers/block/cciss.c~cciss_busy_conf_for_lx2619-rc4 > > > > drivers/block/cciss.c > > > > --- linux-2.6/drivers/block/cciss.c~cciss_busy_conf_for_lx2619-rc4 > > > > 2006-11-06 13:27:53.0 -0600 > > > > +++ linux-2.6-root/drivers/block/cciss.c2006-11-06 > > > > 13:27:53.0 -0600 > > > > @@ -1190,8 +1190,11 @@ static void cciss_check_queues(ctlr_info > > > > /* make sure the disk has been added and the drive is > > > > real > > > > * because this can be called from the middle of > > > > init_one. > > > > */ > > > > - if (!(h->drv[curr_queue].queue) || > > > > !(h->drv[curr_queue].heads)) > > > > + if (!(h->drv[curr_queue].queue) || > > > > + !(h->drv[curr_queue].heads) || > > > > + h->drv[curr_queue].busy_configuring) > > > > continue; > > > > + > > > > blk_start_queue(h->gendisk[curr_queue]->queue); > > > > > > This is racy, because you don't start the queue when you unset > > > ->busy_configuring later on. For this to be safe, you need to call > > > blk_start_queue() when you set ->busy_configuring to 0. > > > > Jens, please see Chase's reply to your concerns: > > > busy_configuring - I do not think this is racy. This > > > flag is used only when we are removing/deleting a disk. In > > > this case the queue is cleaned up and the disk is deleted. > > > If we are doing that then there is no queue to start later. > > > The check of this flag in the interrupt handler is to prevent > > > us from trying to start a queue that is in the middle of > > > being deleted. This flag could be called busy_deleting. > > Ok, no worries then if it's simply a going away flag. I wonder if it's > needed at all, but it certainly doesn't hurt. hey Jens, Just a poke since I haven't seen this change go into your block tree. Is it still in-plan? -- dann frazier - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 9/12] repost: cciss: add busy_configuring flag
On Wed, Dec 13, 2006 at 01:52:36PM +0100, Jens Axboe wrote: On Tue, Dec 12 2006, Mike Miller (OS Dev) wrote: On Mon, Nov 06, 2006 at 09:32:00PM +0100, Jens Axboe wrote: On Mon, Nov 06 2006, Mike Miller (OS Dev) wrote: PATCH 9 of 12 This patch adds a check for busy_configuring to prevent starting a queue on a drive that may be in the midst of updating, configuring, deleting, etc. This had a test for if the queue was stopped or plugged but that seemed to cause issues. Please consider this for inclusion. Thanks, mikem Signed-off-by: Mike Miller [EMAIL PROTECTED] --- drivers/block/cciss.c |5 - 1 files changed, 4 insertions(+), 1 deletion(-) diff -puN drivers/block/cciss.c~cciss_busy_conf_for_lx2619-rc4 drivers/block/cciss.c --- linux-2.6/drivers/block/cciss.c~cciss_busy_conf_for_lx2619-rc4 2006-11-06 13:27:53.0 -0600 +++ linux-2.6-root/drivers/block/cciss.c2006-11-06 13:27:53.0 -0600 @@ -1190,8 +1190,11 @@ static void cciss_check_queues(ctlr_info /* make sure the disk has been added and the drive is real * because this can be called from the middle of init_one. */ - if (!(h-drv[curr_queue].queue) || !(h-drv[curr_queue].heads)) + if (!(h-drv[curr_queue].queue) || + !(h-drv[curr_queue].heads) || + h-drv[curr_queue].busy_configuring) continue; + blk_start_queue(h-gendisk[curr_queue]-queue); This is racy, because you don't start the queue when you unset -busy_configuring later on. For this to be safe, you need to call blk_start_queue() when you set -busy_configuring to 0. Jens, please see Chase's reply to your concerns: busy_configuring - I do not think this is racy. This flag is used only when we are removing/deleting a disk. In this case the queue is cleaned up and the disk is deleted. If we are doing that then there is no queue to start later. The check of this flag in the interrupt handler is to prevent us from trying to start a queue that is in the middle of being deleted. This flag could be called busy_deleting. Ok, no worries then if it's simply a going away flag. I wonder if it's needed at all, but it certainly doesn't hurt. hey Jens, Just a poke since I haven't seen this change go into your block tree. Is it still in-plan? -- dann frazier - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 9/12] repost: cciss: add busy_configuring flag
On Tue, Dec 12 2006, Mike Miller (OS Dev) wrote: > On Mon, Nov 06, 2006 at 09:32:00PM +0100, Jens Axboe wrote: > > On Mon, Nov 06 2006, Mike Miller (OS Dev) wrote: > > > PATCH 9 of 12 > > > > > > This patch adds a check for busy_configuring to prevent starting a queue > > > on a drive that may be in the midst of updating, configuring, deleting, > > > etc. > > > > > > This had a test for if the queue was stopped or plugged but that seemed > > > to cause issues. > > > Please consider this for inclusion. > > > > > > Thanks, > > > mikem > > > > > > Signed-off-by: Mike Miller <[EMAIL PROTECTED]> > > > > > > > > > > > > --- > > > > > > drivers/block/cciss.c |5 - > > > 1 files changed, 4 insertions(+), 1 deletion(-) > > > > > > diff -puN drivers/block/cciss.c~cciss_busy_conf_for_lx2619-rc4 > > > drivers/block/cciss.c > > > --- linux-2.6/drivers/block/cciss.c~cciss_busy_conf_for_lx2619-rc4 > > > 2006-11-06 13:27:53.0 -0600 > > > +++ linux-2.6-root/drivers/block/cciss.c 2006-11-06 13:27:53.0 > > > -0600 > > > @@ -1190,8 +1190,11 @@ static void cciss_check_queues(ctlr_info > > > /* make sure the disk has been added and the drive is real > > >* because this can be called from the middle of init_one. > > >*/ > > > - if (!(h->drv[curr_queue].queue) || !(h->drv[curr_queue].heads)) > > > + if (!(h->drv[curr_queue].queue) || > > > + !(h->drv[curr_queue].heads) || > > > + h->drv[curr_queue].busy_configuring) > > > continue; > > > + > > > blk_start_queue(h->gendisk[curr_queue]->queue); > > > > This is racy, because you don't start the queue when you unset > > ->busy_configuring later on. For this to be safe, you need to call > > blk_start_queue() when you set ->busy_configuring to 0. > > Jens, please see Chase's reply to your concerns: > > busy_configuring - I do not think this is racy. This > > flag is used only when we are removing/deleting a disk. In > > this case the queue is cleaned up and the disk is deleted. > > If we are doing that then there is no queue to start later. > > The check of this flag in the interrupt handler is to prevent > > us from trying to start a queue that is in the middle of > > being deleted. This flag could be called busy_deleting. Ok, no worries then if it's simply a going away flag. I wonder if it's needed at all, but it certainly doesn't hurt. -- Jens Axboe - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 9/12] repost: cciss: add busy_configuring flag
On Tue, Dec 12 2006, Mike Miller (OS Dev) wrote: On Mon, Nov 06, 2006 at 09:32:00PM +0100, Jens Axboe wrote: On Mon, Nov 06 2006, Mike Miller (OS Dev) wrote: PATCH 9 of 12 This patch adds a check for busy_configuring to prevent starting a queue on a drive that may be in the midst of updating, configuring, deleting, etc. This had a test for if the queue was stopped or plugged but that seemed to cause issues. Please consider this for inclusion. Thanks, mikem Signed-off-by: Mike Miller [EMAIL PROTECTED] --- drivers/block/cciss.c |5 - 1 files changed, 4 insertions(+), 1 deletion(-) diff -puN drivers/block/cciss.c~cciss_busy_conf_for_lx2619-rc4 drivers/block/cciss.c --- linux-2.6/drivers/block/cciss.c~cciss_busy_conf_for_lx2619-rc4 2006-11-06 13:27:53.0 -0600 +++ linux-2.6-root/drivers/block/cciss.c 2006-11-06 13:27:53.0 -0600 @@ -1190,8 +1190,11 @@ static void cciss_check_queues(ctlr_info /* make sure the disk has been added and the drive is real * because this can be called from the middle of init_one. */ - if (!(h-drv[curr_queue].queue) || !(h-drv[curr_queue].heads)) + if (!(h-drv[curr_queue].queue) || + !(h-drv[curr_queue].heads) || + h-drv[curr_queue].busy_configuring) continue; + blk_start_queue(h-gendisk[curr_queue]-queue); This is racy, because you don't start the queue when you unset -busy_configuring later on. For this to be safe, you need to call blk_start_queue() when you set -busy_configuring to 0. Jens, please see Chase's reply to your concerns: busy_configuring - I do not think this is racy. This flag is used only when we are removing/deleting a disk. In this case the queue is cleaned up and the disk is deleted. If we are doing that then there is no queue to start later. The check of this flag in the interrupt handler is to prevent us from trying to start a queue that is in the middle of being deleted. This flag could be called busy_deleting. Ok, no worries then if it's simply a going away flag. I wonder if it's needed at all, but it certainly doesn't hurt. -- Jens Axboe - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 9/12] repost: cciss: add busy_configuring flag
On Mon, Nov 06, 2006 at 09:32:00PM +0100, Jens Axboe wrote: > On Mon, Nov 06 2006, Mike Miller (OS Dev) wrote: > > PATCH 9 of 12 > > > > This patch adds a check for busy_configuring to prevent starting a queue > > on a drive that may be in the midst of updating, configuring, deleting, etc. > > > > This had a test for if the queue was stopped or plugged but that seemed > > to cause issues. > > Please consider this for inclusion. > > > > Thanks, > > mikem > > > > Signed-off-by: Mike Miller <[EMAIL PROTECTED]> > > > > > > > > --- > > > > drivers/block/cciss.c |5 - > > 1 files changed, 4 insertions(+), 1 deletion(-) > > > > diff -puN drivers/block/cciss.c~cciss_busy_conf_for_lx2619-rc4 > > drivers/block/cciss.c > > --- linux-2.6/drivers/block/cciss.c~cciss_busy_conf_for_lx2619-rc4 > > 2006-11-06 13:27:53.0 -0600 > > +++ linux-2.6-root/drivers/block/cciss.c2006-11-06 13:27:53.0 > > -0600 > > @@ -1190,8 +1190,11 @@ static void cciss_check_queues(ctlr_info > > /* make sure the disk has been added and the drive is real > > * because this can be called from the middle of init_one. > > */ > > - if (!(h->drv[curr_queue].queue) || !(h->drv[curr_queue].heads)) > > + if (!(h->drv[curr_queue].queue) || > > + !(h->drv[curr_queue].heads) || > > + h->drv[curr_queue].busy_configuring) > > continue; > > + > > blk_start_queue(h->gendisk[curr_queue]->queue); > > This is racy, because you don't start the queue when you unset > ->busy_configuring later on. For this to be safe, you need to call > blk_start_queue() when you set ->busy_configuring to 0. Jens, please see Chase's reply to your concerns: > busy_configuring - I do not think this is racy. This > flag is used only when we are removing/deleting a disk. In > this case the queue is cleaned up and the disk is deleted. > If we are doing that then there is no queue to start later. > The check of this flag in the interrupt handler is to prevent > us from trying to start a queue that is in the middle of > being deleted. This flag could be called busy_deleting. > Thanks, mikem, Chase - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 9/12] repost: cciss: add busy_configuring flag
On Mon, Nov 06, 2006 at 09:32:00PM +0100, Jens Axboe wrote: On Mon, Nov 06 2006, Mike Miller (OS Dev) wrote: PATCH 9 of 12 This patch adds a check for busy_configuring to prevent starting a queue on a drive that may be in the midst of updating, configuring, deleting, etc. This had a test for if the queue was stopped or plugged but that seemed to cause issues. Please consider this for inclusion. Thanks, mikem Signed-off-by: Mike Miller [EMAIL PROTECTED] --- drivers/block/cciss.c |5 - 1 files changed, 4 insertions(+), 1 deletion(-) diff -puN drivers/block/cciss.c~cciss_busy_conf_for_lx2619-rc4 drivers/block/cciss.c --- linux-2.6/drivers/block/cciss.c~cciss_busy_conf_for_lx2619-rc4 2006-11-06 13:27:53.0 -0600 +++ linux-2.6-root/drivers/block/cciss.c2006-11-06 13:27:53.0 -0600 @@ -1190,8 +1190,11 @@ static void cciss_check_queues(ctlr_info /* make sure the disk has been added and the drive is real * because this can be called from the middle of init_one. */ - if (!(h-drv[curr_queue].queue) || !(h-drv[curr_queue].heads)) + if (!(h-drv[curr_queue].queue) || + !(h-drv[curr_queue].heads) || + h-drv[curr_queue].busy_configuring) continue; + blk_start_queue(h-gendisk[curr_queue]-queue); This is racy, because you don't start the queue when you unset -busy_configuring later on. For this to be safe, you need to call blk_start_queue() when you set -busy_configuring to 0. Jens, please see Chase's reply to your concerns: busy_configuring - I do not think this is racy. This flag is used only when we are removing/deleting a disk. In this case the queue is cleaned up and the disk is deleted. If we are doing that then there is no queue to start later. The check of this flag in the interrupt handler is to prevent us from trying to start a queue that is in the middle of being deleted. This flag could be called busy_deleting. Thanks, mikem, Chase - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
