From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752711AbcGVISN (ORCPT ); Fri, 22 Jul 2016 04:18:13 -0400 Received: from aserp1040.oracle.com ([141.146.126.69]:35116 "EHLO aserp1040.oracle.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751417AbcGVISI (ORCPT ); Fri, 22 Jul 2016 04:18:08 -0400 Message-ID: <5791D6AC.1070604@oracle.com> Date: Fri, 22 Jul 2016 16:17:48 +0800 From: Bob Liu User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:17.0) Gecko/20130308 Thunderbird/17.0.4 MIME-Version: 1.0 To: =?ISO-8859-1?Q?Roger_Pau_Monn=E9?= CC: linux-kernel@vger.kernel.org, xen-devel@lists.xenproject.org, konrad.wilk@oracle.com, jgross@suse.com Subject: Re: [PATCH 3/3] xen-blkfront: dynamic configuration of per-vbd resources References: <1468575109-12209-1-git-send-email-bob.liu@oracle.com> <1468575109-12209-3-git-send-email-bob.liu@oracle.com> <20160721085756.ps4rtdns4xh35yii@mac> <57909F05.9030809@oracle.com> <20160722074506.l5nfcmqg3jzsmxzi@mac> In-Reply-To: <20160722074506.l5nfcmqg3jzsmxzi@mac> Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: 8bit X-Source-IP: userv0022.oracle.com [156.151.31.74] Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On 07/22/2016 03:45 PM, Roger Pau Monné wrote: > On Thu, Jul 21, 2016 at 06:08:05PM +0800, Bob Liu wrote: >> >> On 07/21/2016 04:57 PM, Roger Pau Monné wrote: ..[snip].. >>>> + >>>> +static ssize_t dynamic_reconfig_device(struct blkfront_info *info, ssize_t count) >>>> +{ >>>> + unsigned int i; >>>> + int err = -EBUSY; >>>> + >>>> + /* >>>> + * Make sure no migration in parallel, device lock is actually a >>>> + * mutex. >>>> + */ >>>> + if (!device_trylock(&info->xbdev->dev)) { >>>> + pr_err("Fail to acquire dev:%s lock, may be in migration.\n", >>>> + dev_name(&info->xbdev->dev)); >>>> + return err; >>>> + } >>>> + >>>> + /* >>>> + * Prevent new requests and guarantee no uncompleted reqs. >>>> + */ >>>> + blk_mq_freeze_queue(info->rq); >>>> + if (part_in_flight(&info->gd->part0)) >>>> + goto out; >>>> + >>>> + /* >>>> + * Front Backend >>>> + * Switch to XenbusStateClosed >>>> + * frontend_changed(): >>>> + * case XenbusStateClosed: >>>> + * xen_blkif_disconnect() >>>> + * Switch to XenbusStateClosed >>>> + * blkfront_resume(): >>>> + * frontend_changed(): >>>> + * reconnect >>>> + * Wait until XenbusStateConnected >>>> + */ >>>> + info->reconfiguring = true; >>>> + xenbus_switch_state(info->xbdev, XenbusStateClosed); >>>> + >>>> + /* Poll every 100ms, 1 minute timeout. */ >>>> + for (i = 0; i < 600; i++) { >>>> + /* >>>> + * Wait backend enter XenbusStateClosed, blkback_changed() >>>> + * will clear reconfiguring. >>>> + */ >>>> + if (!info->reconfiguring) >>>> + goto resume; >>>> + schedule_timeout_interruptible(msecs_to_jiffies(100)); >>>> + } >>> >>> Instead of having this wait, could you just set info->reconfiguring = 1, set >>> the frontend state to XenbusStateClosed and mimic exactly what a resume from >>> suspension does? blkback_changed would have to set the frontend state to >>> InitWait when it detects that the backend has switched to Closed, and call >>> blkfront_resume. >> >> >> I think that won't work. >> In the real "resume" case, the power management system will trigger all ->resume() path. >> But there is no place for dynamic configuration. > > Hello, > > I think it should be possible to set info->reconfiguring and wait for the > backend to switch to state Closed, at that point we should call blkif_resume > (from blkback_changed) and the backend will follow the reconection. > Okay, I get your point. Yes, that's an option. But this will make 'dynamic configuration' to be async, I'm worry about the end-user will get panic. E.g A end-user "echo > /sys/devices/vbd-xxx/max_indirect_segs", but then the device will be Closed and disappeared, the user have to wait for a random time so that the device can resume. -- Regards, -Bob