From mboxrd@z Thu Jan 1 00:00:00 1970 From: Tejun Heo Subject: Re: [PATCH 09/15] libata-hp: activate hotplug by adding a call to ata_eh_hotplug() from EH Date: Thu, 13 Apr 2006 18:30:06 +0900 Message-ID: <443E1A1E.4070705@gmail.com> References: <11447648473100-git-send-email-htejun@gmail.com> <1144916301.19687.22.camel@localhost.localdomain> <443E0FC3.8000808@gmail.com> <1144918803.19687.27.camel@localhost.localdomain> Mime-Version: 1.0 Content-Type: text/plain; charset=EUC-KR Content-Transfer-Encoding: 7bit Return-path: Received: from zproxy.gmail.com ([64.233.162.192]:23509 "EHLO zproxy.gmail.com") by vger.kernel.org with ESMTP id S964842AbWDMJaR (ORCPT ); Thu, 13 Apr 2006 05:30:17 -0400 Received: by zproxy.gmail.com with SMTP id o37so1555079nzf for ; Thu, 13 Apr 2006 02:30:16 -0700 (PDT) In-Reply-To: <1144918803.19687.27.camel@localhost.localdomain> Sender: linux-ide-owner@vger.kernel.org List-Id: linux-ide@vger.kernel.org To: "zhao, forrest" Cc: jgarzik@pobox.com, alan@lxorguk.ukuu.org.uk, axboe@suse.de, albertcc@tw.ibm.com, lkosewsk@gmail.com, linux-ide@vger.kernel.org zhao, forrest wrote: > On Thu, 2006-04-13 at 17:45 +0900, Tejun Heo wrote: >> zhao, forrest wrote: >>> On Tue, 2006-04-11 at 23:14 +0900, Tejun Heo wrote: >>>> Now all the required pieces for hotplug support are in place. Update >>>> all ->error_handler's to call ata_eh_hotplug() after regular EH is >>>> finished. This enables hotplug support for all drivers which use new >>>> EH. With this patch, all new EH drivers can do warm unplug and plug. >>>> >>>> Signed-off-by: Tejun Heo >>>> >>>> --- >>>> >>>> drivers/scsi/ahci.c | 1 + >>>> drivers/scsi/libata-bmdma.c | 1 + >>>> drivers/scsi/sata_sil24.c | 1 + >>>> 3 files changed, 3 insertions(+), 0 deletions(-) >>>> >>>> 5a9a286e07ca0d216be1b8f81ab6c6e7048ae963 >>>> diff --git a/drivers/scsi/ahci.c b/drivers/scsi/ahci.c >>>> index a8b699a..7f478c4 100644 >>>> --- a/drivers/scsi/ahci.c >>>> +++ b/drivers/scsi/ahci.c >>>> @@ -864,6 +864,7 @@ static void ahci_error_handler(struct at >>>> ata_eh_revive(ap, action, >>>> ahci_softreset, ahci_hardreset, ahci_postreset); >>>> ata_eh_finish_qcs(ap, qc, &tf); >>>> + ata_eh_hotplug(ap); >>>> } >>>> >>>> static void ahci_post_internal_cmd(struct ata_queued_cmd *qc) >>>> diff --git a/drivers/scsi/libata-bmdma.c b/drivers/scsi/libata-bmdma.c >>>> index 3c3bc71..a87efae 100644 >>>> --- a/drivers/scsi/libata-bmdma.c >>>> +++ b/drivers/scsi/libata-bmdma.c >>>> @@ -729,6 +729,7 @@ void ata_bmdma_drive_eh(struct ata_port >>>> ata_eh_report(ap, qc, &tf, serror, action, NULL); >>>> ata_eh_revive(ap, action, softreset, hardreset, postreset); >>>> ata_eh_finish_qcs(ap, qc, &tf); >>>> + ata_eh_hotplug(ap); >>>> } >>>> >>>> /** >>>> diff --git a/drivers/scsi/sata_sil24.c b/drivers/scsi/sata_sil24.c >>>> index 9acf7a4..1ca76f7 100644 >>>> --- a/drivers/scsi/sata_sil24.c >>>> +++ b/drivers/scsi/sata_sil24.c >>>> @@ -852,6 +852,7 @@ static void sil24_error_handler(struct a >>>> ata_eh_revive(ap, action, >>>> sil24_softreset, sil24_hardreset, sil24_postreset); >>>> ata_eh_finish_qcs(ap, qc, &tf); >>>> + ata_eh_hotplug(ap); >>>> } >>>> >>> Does it make sense to invoke ata_eh_hotplug() in ata_scsi_error() >>> instead of invoking ata_eh_hotplug() in each LLDD in order to >>> make code more general? >>> >> No, not really. To do that we'll need another port operation >> ->hotplug(), which, I think, has no real benefits. > >>>From my understanding, in ata_scsi_error invoking > ata_eh_hotplug(ap); after ap->ops->error_handler(ap); > will do. No need to add ->hotplug() for port operation. > This way we avoid to invoke ata_eh_hotplug() in each LLDD. > Hmmmm... maybe you're right and we don't need to allow LLDDs to override hotplug operation. After all, LLDDs cannot override ata_bus_probe(). But, then again, it's just nice and consistent to allow overriding of the operation. And, ata_eh_hotplug() fits nicely with other EH operations, IMHO. And calling one more operation isn't really a big deal. Maybe, we need to come up with the function which packages standard calls to eh helpers for drivers like sata_sil24 and ahci, but I think the current level of code duplication is well within acceptable range. Do you have any other reason than duplicated calls to ata_eh_hotplug()? -- tejun