On Thu, Jul 08 2010 at 12:17am -0400, FUJITA Tomonori <fujita.tomonori@xxxxxxxxxxxxx> wrote: > On Tue, 6 Jul 2010 09:59:58 -0400 > Mike Snitzer <snitzer@xxxxxxxxxx> wrote: > > > Here is a minimalist patch that 1) removes the discard request's page > > leak 2) preserves the prep cleanup rules covered above. Fixing the leak > > is a priority, introducing additional error path code sharing/cleanup is > > secondary and can come later. > > This patch doesn't work. I hit kernel oops since now this patch frees > a discard page twice. I load tested my v2 patch quite a bit but didn't hit the oops. Guess you're just lucky ;) > If scsi_init_io hits BLKPREP_DEFER (e.g. due to scsi_init_sgtable), > scsi_setup_blk_pc_cmnd calls scsi_unprep_request. So sd_unprep_fn > frees a page. Then, scsi_setup_discard_cmnd frees the same page again. OK, thanks for catching this. > From: FUJITA Tomonori <fujita.tomonori@xxxxxxxxxxxxx> > Subject: [PATCH] scsi: fix discard page leak > > We leak a page allocated for discard on some error conditions > (e.g. scsi_prep_state_check returns BLKPREP_DEFER in > scsi_setup_blk_pc_cmnd). > > We unprep on requests that weren't prepped in the error path of > scsi_init_io. It makes the error path to clean up scsi commands messy. > > Let's strictly apply the rule that we can't unprep on a request that > wasn't prepped. > > Calling just scsi_put_command() in the error path of scsi_init_io() is > enough. We don't set REQ_DONTPREP yet. > > scsi_setup_discard_cmnd can safely free a page on the error case with > the above rule. > > Signed-off-by: FUJITA Tomonori <fujita.tomonori@xxxxxxxxxxxxx> Acked-by: Mike Snitzer <snitzer@xxxxxxxxxx> Jens, it'd be great if we could get this fix in now. -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
