Re: [PATCH v3 2/4] cxlflash: Base error recovery support

2015-08-06 Thread Daniel Axtens
Hi, 

> @@ -1857,9 +1884,18 @@ static int cxlflash_eh_device_reset_handler(struct 
> scsi_cmnd *scp)
>get_unaligned_be32(&((u32 *)scp->cmnd)[2]),
>get_unaligned_be32(&((u32 *)scp->cmnd)[3]));
>  
> - rcr = send_tmf(afu, scp, TMF_LUN_RESET);
> - if (unlikely(rcr))
> - rc = FAILED;
> + switch (cfg->eeh_active) {
> + case EEH_STATE_NONE:
> + rcr = send_tmf(afu, scp, TMF_LUN_RESET);
> + if (unlikely(rcr))
> + rc = FAILED;
> + break;
> + case EEH_STATE_ACTIVE:
> + wait_event(cfg->eeh_waitq, cfg->eeh_active != EEH_STATE_ACTIVE);
> + break;
> + case EEH_STATE_FAILED:
> + break;
> + }
>  

Looking at the context here, it looks like rc gets initalised to
SUCCESS. In that case, in the EEH failed case, you'll return SUCCESS.
I'm not particularly clear on the semantics here: does that make sense?

Likewise, in the EEH active state, when you finish the wait_event,
should you check if the EEH state went to NONE or FAILED before you
break?

There's a similar case below in host_reset.

>   pr_debug("%s: returning rc=%d\n", __func__, rc);
>   return rc;
> @@ -1889,11 +1925,23 @@ static int cxlflash_eh_host_reset_handler(struct 
> scsi_cmnd *scp)
>get_unaligned_be32(&((u32 *)scp->cmnd)[2]),
>get_unaligned_be32(&((u32 *)scp->cmnd)[3]));
>  
> - rcr = afu_reset(cfg);
> - if (rcr == 0)
> - rc = SUCCESS;
> - else
> - rc = FAILED;
> + switch (cfg->eeh_active) {
> + case EEH_STATE_NONE:
> + cfg->eeh_active = EEH_STATE_FAILED;
> + rcr = afu_reset(cfg);
> + if (rcr == 0)
> + rc = SUCCESS;
> + else
> + rc = FAILED;
> + cfg->eeh_active = EEH_STATE_NONE;
> + wake_up_all(&cfg->eeh_waitq);
> + break;
> + case EEH_STATE_ACTIVE:
> + wait_event(cfg->eeh_waitq, cfg->eeh_active != EEH_STATE_ACTIVE);
> + break;
> + case EEH_STATE_FAILED:
> + break;
> + }
>  
>   pr_debug("%s: returning rc=%d\n", __func__, rc);
>   return rc;
> @@ -2145,6 +2193,11 @@ static void cxlflash_worker_thread(struct work_struct 
> *work)
>   int port;
>   ulong lock_flags;
>  
> + /* Avoid MMIO if the device has failed */
> +
> + if (cfg->eeh_active == EEH_STATE_FAILED)
> + return;
> +
Should this check be != EEH_STATE_NONE? Or is this called within the
error recovery process?

>   spin_lock_irqsave(cfg->host->host_lock, lock_flags);
>  
>   if (cfg->lr_state == LINK_RESET_REQUIRED) {
> @@ -2226,6 +2279,8 @@ static int cxlflash_probe(struct pci_dev *pdev,
>  
>   cfg->init_state = INIT_STATE_NONE;
>   cfg->dev = pdev;
> +
> + cfg->eeh_active = EEH_STATE_NONE;
>   cfg->dev_id = (struct pci_device_id *)dev_id;
>  
> 
> @@ -2286,6 +2341,85 @@ out_remove:
>   goto out;
>  }
>  
> +/**
> + * cxlflash_pci_error_detected() - called when a PCI error is detected
> + * @pdev:PCI device struct.
> + * @state:   PCI channel state.
> + *
> + * Return: PCI_ERS_RESULT_NEED_RESET or PCI_ERS_RESULT_DISCONNECT
> + */
> +static pci_ers_result_t cxlflash_pci_error_detected(struct pci_dev *pdev,
> + pci_channel_state_t state)
> +{
> + struct cxlflash_cfg *cfg = pci_get_drvdata(pdev);
> +
> + pr_debug("%s: pdev=%p state=%u\n", __func__, pdev, state);
> +
> + switch (state) {
> + case pci_channel_io_frozen:
> + cfg->eeh_active = EEH_STATE_ACTIVE;
> + udelay(100);
> +
> + term_mc(cfg, UNDO_START);
> + stop_afu(cfg);
> +
> + return PCI_ERS_RESULT_CAN_RECOVER;

I think that should PCI_ERS_RESULT_NEED_RESET.

Apart from that, it's looking pretty good from an EEH perspective.

-- 
Regards,
Daniel


signature.asc
Description: This is a digitally signed message part


Re: [dm-devel] kernel BUG at drivers/scsi/scsi_lib.c:1101! observed during md5sum for one file on (RAID4->RAID0) device

2015-08-06 Thread NeilBrown
On Thu, 6 Aug 2015 02:52:45 -0400 (EDT) Yi Zhang 
wrote:

> Hi Neil
> I test 10 times with below patch on Linux 4.2-rc5, didn't reproduce the 
> issue, thanks.
> 
> 

Thanks for the confirmation.
I will be submitting it for 4.3 and then it will flow into -stable
kernels.

NeilBrown
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH v3 3/4] cxlflash: Superpipe support

2015-08-06 Thread Manoj Kumar

Brian: Thanks for your review. See comments inline, below.

- Manoj Kumar

On 8/6/2015 3:46 PM, Brian King wrote:

   * cxlflash_queuecommand() - sends a mid-layer request
   * @host: SCSI host associated with device.
   * @scp:  SCSI command to send.
@@ -512,6 +535,13 @@ static int cxlflash_queuecommand(struct Scsi_Host *host, 
struct scsi_cmnd *scp)
 get_unaligned_be32(&((u32 *)scp->cmnd)[2]),
 get_unaligned_be32(&((u32 *)scp->cmnd)[3]));

+   /* Fail all read/write commands when in operating superpipe mode */
+   if (scp->device->hostdata && is_scsi_read_write(scp)) {
+   pr_debug_ratelimited("%s: LUN being used in superpipe mode. "
+"Operation not allowed!\n", __func__);
+   goto error;
+   }


Not sure I like this. A couple of concerns:

1. Any process that comes along and issues a read to the device will result in 
I/O errors getting logged
by the layers above you. The general expectation is that if reads or writes 
are failing on a block device,
then something is wrong and the user should know about it. A user innocently running 
"fdisk -l", for example,
to list all disk partitions on the system, would see errors getting logged 
for every disk configured for
superpipe mode.
2. How will users know that devices are in superpipe mode? The only indication 
is the driver specific sysfs attribute
that no existing tooling will be checking. GUI tools to do disk 
partitioning and such will see these devices
and present them to the user with the expectation that something can be 
done with them.
3. If this is a multipath device, you'll have a dm device sitting on top of 
this and potentially have multipathd doing health
checking periodically and these devices will show up in multipath -ll 
output.

It seems to me like the cleanest option would be, when switching into superpipe 
mode, for the user code doing
this to unbind the sd driver from the relevant scsi device. This will prevent 
any reads or writes from being
issued to the LUN from the block layer, since there will no longer be any way 
to do this. It will also prevent
these devices from showing up in GUIs and menus where we don't want them to.

So, I'd say, kill this snooping of the op code and failing I/O in superpipe 
mode, and solve this in userspace.


The intent of this snooping and failing I/O was to prevent an accidental 
over-write of the LUN in superpipe mode by some other user unaware of 
our sysfs attribute. This was an additional precautionary measure to 
unbinding the sd driver from the relevant scsi device. We can fall back 
on to that primary mechanism.




+   gli->max_lba = swab64(*((u64 *)&cmd_buf[0]));
+   gli->blk_len = swab32(*((u32 *)&cmd_buf[8]));


This doesn't look right. How does this work on big endian? I think you want to 
be using
be64_to_cpu and be32_to_cpu here.



Good catch. Will rectify in the v4 patch.

--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH v3 3/4] cxlflash: Superpipe support

2015-08-06 Thread Brian King
On 08/02/2015 11:33 PM, Matthew R. Ochs wrote:
> diff --git a/drivers/scsi/cxlflash/main.c b/drivers/scsi/cxlflash/main.c
> index 9515525..196c651 100644
> --- a/drivers/scsi/cxlflash/main.c
> +++ b/drivers/scsi/cxlflash/main.c
> @@ -484,6 +484,29 @@ static const char *cxlflash_driver_info(struct Scsi_Host 
> *host)
>  }
> 
>  /**
> + * is_scsi_read_write() - evaluates if a SCSI command is read or write
> + * @scp: SCSI command to evaluate.
> + *
> + * Return: true or false
> + */
> +static inline bool is_scsi_read_write(struct scsi_cmnd *scp)
> +{
> + switch (scp->cmnd[0]) {
> + case READ_6:
> + case READ_10:
> + case READ_12:
> + case READ_16:
> + case WRITE_6:
> + case WRITE_10:
> + case WRITE_12:
> + case WRITE_16:
> + return true;
> + }
> +
> + return false;
> +}
> +
> +/**
>   * cxlflash_queuecommand() - sends a mid-layer request
>   * @host:SCSI host associated with device.
>   * @scp: SCSI command to send.
> @@ -512,6 +535,13 @@ static int cxlflash_queuecommand(struct Scsi_Host *host, 
> struct scsi_cmnd *scp)
>get_unaligned_be32(&((u32 *)scp->cmnd)[2]),
>get_unaligned_be32(&((u32 *)scp->cmnd)[3]));
> 
> + /* Fail all read/write commands when in operating superpipe mode */
> + if (scp->device->hostdata && is_scsi_read_write(scp)) {
> + pr_debug_ratelimited("%s: LUN being used in superpipe mode. "
> +  "Operation not allowed!\n", __func__);
> + goto error;
> + }

Not sure I like this. A couple of concerns:

1. Any process that comes along and issues a read to the device will result in 
I/O errors getting logged
   by the layers above you. The general expectation is that if reads or writes 
are failing on a block device,
   then something is wrong and the user should know about it. A user innocently 
running "fdisk -l", for example,
   to list all disk partitions on the system, would see errors getting logged 
for every disk configured for
   superpipe mode.
2. How will users know that devices are in superpipe mode? The only indication 
is the driver specific sysfs attribute
   that no existing tooling will be checking. GUI tools to do disk partitioning 
and such will see these devices
   and present them to the user with the expectation that something can be done 
with them.
3. If this is a multipath device, you'll have a dm device sitting on top of 
this and potentially have multipathd doing health
   checking periodically and these devices will show up in multipath -ll output.

It seems to me like the cleanest option would be, when switching into superpipe 
mode, for the user code doing
this to unbind the sd driver from the relevant scsi device. This will prevent 
any reads or writes from being
issued to the LUN from the block layer, since there will no longer be any way 
to do this. It will also prevent
these devices from showing up in GUIs and menus where we don't want them to.

So, I'd say, kill this snooping of the op code and failing I/O in superpipe 
mode, and solve this in userspace.


> +
>   /*
>* If a Task Management Function is active, wait for it to complete
>* before continuing with regular commands.
> @@ -531,7 +561,7 @@ static int cxlflash_queuecommand(struct Scsi_Host *host, 
> struct scsi_cmnd *scp)
>   goto out;
>   case EEH_STATE_FAILED:
>   pr_debug_ratelimited("%s: device has failed!\n", __func__);
> - goto out;
> + goto error;
>   case EEH_STATE_NONE:
>   break;
>   }
> @@ -584,6 +614,10 @@ static int cxlflash_queuecommand(struct Scsi_Host *host, 
> struct scsi_cmnd *scp)
> 
>  out:
>   return rc;
> +error:
> + scp->result = (DID_NO_CONNECT << 16);
> + scp->scsi_done(scp);
> + return 0;
>  }
> 
>  /**

> +
> +/**
> + * read_cap16() - issues a SCSI READ_CAP16 command
> + * @sdev:SCSI device associated with LUN.
> + * @lli: LUN destined for capacity request.
> + *
> + * Return: 0 on success, -errno on failure
> + */
> +static int read_cap16(struct scsi_device *sdev, struct llun_info *lli)
> +{
> + struct glun_info *gli = lli->parent;
> + u8 *buf = NULL;
> + u8 *cmd_buf = NULL;
> + u8 *scsi_cmd = NULL;
> + u8 *sense_buf = NULL;
> + int rc = 0;
> + int result = 0;
> + int retry_cnt = 0;
> + u32 tout = (MC_DISCOVERY_TIMEOUT * HZ);
> + size_t size;
> +
> + size = CMD_BUFSIZE + MAX_COMMAND_SIZE + SCSI_SENSE_BUFFERSIZE;
> +retry:
> + buf = kzalloc(size, GFP_KERNEL);
> + if (unlikely(!buf)) {
> + rc = -ENOMEM;
> + goto out;
> + }
> +
> + cmd_buf = buf;
> + scsi_cmd = cmd_buf + CMD_BUFSIZE;
> + sense_buf = scsi_cmd + MAX_COMMAND_SIZE;
> +
> + scsi_cmd[0] = SERVICE_ACTION_IN_16; /* read cap(16) */
> + scsi_cmd[1] = SAI_READ_CAPACITY_16; /* service action */
> + put_unaligned

Re: [Xen-devel] [PATCH v2 4/8] xen: Use the correctly the Xen memory terminologies

2015-08-06 Thread Stefano Stabellini
On Thu, 6 Aug 2015, Julien Grall wrote:
> On 06/08/15 12:06, Stefano Stabellini wrote:
> > On Thu, 6 Aug 2015, Julien Grall wrote:
> >> Hi,
> >>
> >>
> >> On 04/08/15 19:12, Julien Grall wrote:
> >>> diff --git a/include/xen/page.h b/include/xen/page.h
> >>> index c5ed20b..e7e1425 100644
> >>> --- a/include/xen/page.h
> >>> +++ b/include/xen/page.h
> >>> @@ -3,9 +3,9 @@
> >>>  
> >>>  #include 
> >>>  
> >>> -static inline unsigned long page_to_mfn(struct page *page)
> >>> +static inline unsigned long page_to_gfn(struct page *page)
> >>>  {
> >>> - return pfn_to_mfn(page_to_pfn(page));
> >>> + return pfn_to_gfn(page_to_pfn(page));
> >>>  }
> >>
> >> I've just noticed that there is a function gfn_to_page used for KVM.
> >>
> >> Maybe I should rename page_to_gfn to xen_page_to_gfn to avoid confusion
> >> with KVM one?
> > 
> > Yeah, prepending xen would help to avoid namespace pollution.
> 
> Will do. May I keep your Reviewed-by for this mechanical change?

Yes

--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 5/6] dm: split out a helper to find the ioctl target

2015-08-06 Thread Christoph Hellwig
On Tue, Aug 04, 2015 at 02:05:59PM -0400, Mike Snitzer wrote:
> static int dm_get_live_table_for_ioctl(struct mapped_device *md, int 
> *srcu_idx,
>struct dm_target **tgt, struct block_device **bdev, fmode_t *mode)
> 
> Otherwise, looks good.

I'll update it.
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: Persistent Reservation API

2015-08-06 Thread Christoph Hellwig
On Tue, Aug 04, 2015 at 06:04:04PM +, Keith Busch wrote:
> On Tue, 4 Aug 2015, Christoph Hellwig wrote:
>> NVMe support currently isn't included as I don't have a multihost
>> NVMe setup to test on, but if I can find a volunteer to test it I'm
>> happy to write the code for it.
>
> Looks pretty good so far. I'd be happy to give try it out with NVMe
> subsystems.

Thanks, I'll prepare a version once I get a few free minutes.
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 4/6] dm: refactor ioctl handling

2015-08-06 Thread Christoph Hellwig
On Tue, Aug 04, 2015 at 01:56:16PM -0400, Mike Snitzer wrote:
> This should be renamed to dm_prepare_ioctl_fn and the targets' hook
> would be .prepare_ioctl
> 
> Open to other names but if the targets no longer issue the ioctl there
> is little point to call it .ioctl

Sure.  I had ioctl_allowed first but that sounded so ugly that I
undid the renaming..
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: Persistent Reservation API

2015-08-06 Thread Christoph Hellwig
On Tue, Aug 04, 2015 at 01:53:49PM -0400, Mike Snitzer wrote:
> The DM changes need to go through linux-dm.git.  Once the block and SCSI
> bits land I'll rebase accordingly.  That cross-maintainer logisitics
> aside, I'll reply with feedback on the DM patches.

That sounds fine to me, thanks!
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH v3] mpt2sas: setpci reset kernel oops fix

2015-08-06 Thread Johannes Thumshirn
Nagarajkumar Narayanan  writes:

> Can you please review the patch
>
> Thanks,
> Nagaraj
>
> On Tue, Jul 21, 2015 at 10:11 AM, Nagarajkumar Narayanan
>  wrote:
>> I have fixed the lock imbalance could anyone please review the patch
>>
>> Thanks,
>> Nagarajkumar Narayanan
>>
>>
>> On Fri, Jul 17, 2015 at 11:55 AM, Nagarajkumar Narayanan
>>  wrote:
>>> Patch Description:
>>>
>>> In mpt2sas driver due to lack of synchronization between ioctl,
>>> BRM status access through sysfs, pci resource removal kernel oops
>>> happen as ioctl path and BRM status sysfs access path still tries
>>> to access the removed resources
>>>
>>> kernel: BUG: unable to handle kernel paging request at c900171e
>>>
>>> Oops:  [#1] SMP
>>>
>>> Two locks added to provide syncrhonization
>>>
>>> 1. pci_access_mutex: Mutex to synchronize ioctl,sysfs show path and
>>> pci resource handling. PCI resource freeing will lead to free
>>> vital hardware/memory resource, which might be in use by cli/sysfs
>>> path functions resulting in Null pointer reference followed by kernel
>>> crash. To avoid the above race condition we use mutex syncrhonization
>>> which ensures the syncrhonization between cli/sysfs_show path
>>>
>>> 2. spinlock on list operations over IOCs
>>>
>>> Case: when multiple warpdrive cards(IOCs) are in use
>>> Each IOC will added to the ioc list stucture on initialization.
>>> Watchdog threads run at regular intervals to check IOC for any
>>> fault conditions which will trigger the dead_ioc thread to
>>> deallocate pci resource, resulting deleting the IOC netry from list,
>>> this deletion need to protected by spinlock to enusre that
>>> ioc removal is syncrhonized, if not synchronized it might lead to
>>> list_del corruption as the ioc list is traversed in cli path
>>>

Why is the detailed description not included in the patch? This probably
should also be tagged for stable inclusion.

>>>
>>> From 9e54273555d9a34d0d375b91d41318b4ac69a13b Mon Sep 17 00:00:00 2001
>>> From: Nagarajkumar Narayanan 
>>> Date: Fri, 17 Jul 2015 11:28:27 +0530
>>> Subject: [PATCH] mpt2sas setpci reset oops fix
>>>
>>> setpci reset on nytro warpdrive card along with sysfs access and
>>> cli ioctl access resulted in kernel oops
>>>
>>> 1. pci_access_mutex lock added to provide synchronization between IOCTL,
>>>sysfs, PCI resource handling path
>>>
>>> 2. gioc_lock spinlock to protect list operations over multiple
>>>controllers
>>>
>>> Signed-off-by: Nagarajkumar Narayanan 
>>> ---
>>> * v3
>>> - fixed lock imbalance, moved acquiring mutex lock out of if condition
>>>
>>> * v2
>>> - removed is_warpdrive condition for pci_access_mutex lock
>>>
>>> * v1
>>> - using DEFINE_SPINLOCK() to initialize the lock at compile time instead
>>>   of using spin_lock_init
>>>
>>>  drivers/scsi/mpt2sas/mpt2sas_base.c  |7 +++
>>>  drivers/scsi/mpt2sas/mpt2sas_base.h  |   19 ++-
>>>  drivers/scsi/mpt2sas/mpt2sas_ctl.c   |   33 
>>> +
>>>  drivers/scsi/mpt2sas/mpt2sas_scsih.c |   15 ++-
>>>  4 files changed, 68 insertions(+), 6 deletions(-)
>>>
>>> diff --git a/drivers/scsi/mpt2sas/mpt2sas_base.c 
>>> b/drivers/scsi/mpt2sas/mpt2sas_base.c
>>> index 11248de..c0d36b3 100644
>>> --- a/drivers/scsi/mpt2sas/mpt2sas_base.c
>>> +++ b/drivers/scsi/mpt2sas/mpt2sas_base.c
>>> @@ -108,13 +108,17 @@ _scsih_set_fwfault_debug(const char *val, struct 
>>> kernel_param *kp)
>>>  {
>>> int ret = param_set_int(val, kp);
>>> struct MPT2SAS_ADAPTER *ioc;
>>> +   unsigned long flags;
>>>
>>> if (ret)
>>> return ret;
>>>
>>> +   /* global ioc spinlock to protect controller list on list 
>>> operations */
>>> printk(KERN_INFO "setting fwfault_debug(%d)\n", 
>>> mpt2sas_fwfault_debug);
>>> +   spin_lock_irqsave(&gioc_lock, flags);
>>> list_for_each_entry(ioc, &mpt2sas_ioc_list, list)
>>> ioc->fwfault_debug = mpt2sas_fwfault_debug;
>>> +   spin_unlock_irqrestore(&gioc_lock, flags);
>>> return 0;
>>>  }
>>>

I don't think you need the irqsave version here, as mpt2sas_ioc_list is
never touched in an IRQ context. But feel free to prove me wrong.

>>> @@ -4435,6 +4439,8 @@ mpt2sas_base_free_resources(struct MPT2SAS_ADAPTER 
>>> *ioc)
>>> dexitprintk(ioc, printk(MPT2SAS_INFO_FMT "%s\n", ioc->name,
>>> __func__));
>>>
>>> +   /* synchronizing freeing resource with pci_access_mutex lock */
>>> +   mutex_lock(&ioc->pci_access_mutex);
>>> if (ioc->chip_phys && ioc->chip) {
>>> _base_mask_interrupts(ioc);
>>> ioc->shost_recovery = 1;
>>> @@ -4454,6 +4460,7 @@ mpt2sas_base_free_resources(struct MPT2SAS_ADAPTER 
>>> *ioc)
>>> pci_disable_pcie_error_reporting(pdev);
>>> pci_disable_device(pdev);
>>> }
>>> +   mutex_unlock(&ioc->pci_access_mutex);
>>> return;
>>>  }
>>>
>>> diff --git a/drivers/scsi/mpt2sas/mpt2s

Re: [Xen-devel] [PATCH v2 4/8] xen: Use the correctly the Xen memory terminologies

2015-08-06 Thread Julien Grall
On 06/08/15 12:06, Stefano Stabellini wrote:
> On Thu, 6 Aug 2015, Julien Grall wrote:
>> Hi,
>>
>>
>> On 04/08/15 19:12, Julien Grall wrote:
>>> diff --git a/include/xen/page.h b/include/xen/page.h
>>> index c5ed20b..e7e1425 100644
>>> --- a/include/xen/page.h
>>> +++ b/include/xen/page.h
>>> @@ -3,9 +3,9 @@
>>>  
>>>  #include 
>>>  
>>> -static inline unsigned long page_to_mfn(struct page *page)
>>> +static inline unsigned long page_to_gfn(struct page *page)
>>>  {
>>> -   return pfn_to_mfn(page_to_pfn(page));
>>> +   return pfn_to_gfn(page_to_pfn(page));
>>>  }
>>
>> I've just noticed that there is a function gfn_to_page used for KVM.
>>
>> Maybe I should rename page_to_gfn to xen_page_to_gfn to avoid confusion
>> with KVM one?
> 
> Yeah, prepending xen would help to avoid namespace pollution.

Will do. May I keep your Reviewed-by for this mechanical change?

Regards,

-- 
Julien Grall
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [Xen-devel] [PATCH v2 4/8] xen: Use the correctly the Xen memory terminologies

2015-08-06 Thread Stefano Stabellini
On Thu, 6 Aug 2015, Julien Grall wrote:
> Hi,
> 
> 
> On 04/08/15 19:12, Julien Grall wrote:
> > diff --git a/include/xen/page.h b/include/xen/page.h
> > index c5ed20b..e7e1425 100644
> > --- a/include/xen/page.h
> > +++ b/include/xen/page.h
> > @@ -3,9 +3,9 @@
> >  
> >  #include 
> >  
> > -static inline unsigned long page_to_mfn(struct page *page)
> > +static inline unsigned long page_to_gfn(struct page *page)
> >  {
> > -   return pfn_to_mfn(page_to_pfn(page));
> > +   return pfn_to_gfn(page_to_pfn(page));
> >  }
> 
> I've just noticed that there is a function gfn_to_page used for KVM.
> 
> Maybe I should rename page_to_gfn to xen_page_to_gfn to avoid confusion
> with KVM one?

Yeah, prepending xen would help to avoid namespace pollution.
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [Xen-devel] [PATCH v2 4/8] xen: Use the correctly the Xen memory terminologies

2015-08-06 Thread Julien Grall
Hi,


On 04/08/15 19:12, Julien Grall wrote:
> diff --git a/include/xen/page.h b/include/xen/page.h
> index c5ed20b..e7e1425 100644
> --- a/include/xen/page.h
> +++ b/include/xen/page.h
> @@ -3,9 +3,9 @@
>  
>  #include 
>  
> -static inline unsigned long page_to_mfn(struct page *page)
> +static inline unsigned long page_to_gfn(struct page *page)
>  {
> - return pfn_to_mfn(page_to_pfn(page));
> + return pfn_to_gfn(page_to_pfn(page));
>  }

I've just noticed that there is a function gfn_to_page used for KVM.

Maybe I should rename page_to_gfn to xen_page_to_gfn to avoid confusion
with KVM one?

Regards,

-- 
Julien Grall
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html