Messages in this thread | | | From | Tao Guo <> | Date | Fri, 6 Apr 2012 09:31:44 -0700 | Subject | Re: [PATCH] umem: fix up unplugging. |
| |
On Thu, Apr 5, 2012 at 9:19 PM, Jens Axboe <axboe@kernel.dk> wrote: > On 2012-04-05 22:09, Tao Guo wrote: >> On Thu, Apr 5, 2012 at 4:51 PM, Jens Axboe <axboe@kernel.dk> wrote: >>> On 2012-04-04 17:58, Tao Guo wrote: >>>> On Wed, Apr 4, 2012 at 4:18 PM, Jens Axboe <axboe@kernel.dk> wrote: >>>>> On 2012-04-04 16:20, Tao Guo wrote: >>>>>> Hi Andrew, >>>>>> >>>>>> Thanks for your reply. >>>>>> >>>>>> Yes, without this patch the umem driver just doesn't work. >>>>>> It is a bug introduced by commit 7eaceaccab5f40bbfda044629a6298616aeaed50. >>>>>> In that patch, Jens removed the whole mm_unplug_device() function, >>>>>> which used to be >>>>>> the trigger to make umem start to work. >>>>> >>>>> Hmm indeed, that's isn't terribly useful. Why aren't we just calling >>>>> activate_card() on addition of a bio? >>>>> >>>> It is the original design idea, the umem driver also wants to do batch >>>> IO the same as >>>> hard disk drive to fully utilize the IO bandwidth. >>> >>> Sure, just wondering whether the benefits have been tested. But yes, it >>> certainly makes sense. >>> >>> Your patch looks buggy, though. What if the card is currently plugged on >>> another tasks plug list? You seem to assume that it can only be plugged >>> once. >> No, it will register in its own plug callback list successfully, so >> the unplug functions >> will be called separately(twice the same). > > So the code loops through the current plug, checking if it's already on > that list. If not, then it adds it. But what if it's on another plug > list? The above loop could be done with just a > > if (!list_empty(&card->plug_cb.list)) > ... > > instead. Right now you could have !list_empty(&card->plug_cb.list) but > still add it to a different list. > mmm, I was stupid here: card->plug_cb can not be in two list the same time. But I still need to make sure the final unplug will trigger the activate() function. I will sent another version to fix this problem.
Thanks, -Tao > -- > Jens Axboe > -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
| |