From: Wei Yongjun <[email protected]>
Fix to return -ENOMEM in the resource alloc error handling
case instead of 0, as done elsewhere in this function.
Signed-off-by: Wei Yongjun <[email protected]>
---
drivers/staging/vme/devices/vme_user.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/drivers/staging/vme/devices/vme_user.c b/drivers/staging/vme/devices/vme_user.c
index da7f759..5fe224d3 100644
--- a/drivers/staging/vme/devices/vme_user.c
+++ b/drivers/staging/vme/devices/vme_user.c
@@ -734,6 +734,7 @@ static int vme_user_probe(struct vme_dev *vdev)
if (image[i].resource == NULL) {
dev_warn(&vdev->dev,
"Unable to allocate slave resource\n");
+ err = -ENOMEM;
goto err_slave;
}
image[i].size_buf = PCI_BUF_SIZE;
@@ -760,6 +761,7 @@ static int vme_user_probe(struct vme_dev *vdev)
if (image[i].resource == NULL) {
dev_warn(&vdev->dev,
"Unable to allocate master resource\n");
+ err = -ENOMEM;
goto err_master;
}
image[i].size_buf = PCI_BUF_SIZE;
On 13/05/13 07:05, Wei Yongjun wrote:
> From: Wei Yongjun <[email protected]>
>
> Fix to return -ENOMEM in the resource alloc error handling
> case instead of 0, as done elsewhere in this function.
>
Hi Wei,
Thanks for your patch. As this is resource allocation rather than memory
allocation that is failing, would -EAGAIN not make more sense than -ENOMEM?
Martyn
> Signed-off-by: Wei Yongjun <[email protected]>
> ---
> drivers/staging/vme/devices/vme_user.c | 2 ++
> 1 file changed, 2 insertions(+)
>
> diff --git a/drivers/staging/vme/devices/vme_user.c b/drivers/staging/vme/devices/vme_user.c
> index da7f759..5fe224d3 100644
> --- a/drivers/staging/vme/devices/vme_user.c
> +++ b/drivers/staging/vme/devices/vme_user.c
> @@ -734,6 +734,7 @@ static int vme_user_probe(struct vme_dev *vdev)
> if (image[i].resource == NULL) {
> dev_warn(&vdev->dev,
> "Unable to allocate slave resource\n");
> + err = -ENOMEM;
> goto err_slave;
> }
> image[i].size_buf = PCI_BUF_SIZE;
> @@ -760,6 +761,7 @@ static int vme_user_probe(struct vme_dev *vdev)
> if (image[i].resource == NULL) {
> dev_warn(&vdev->dev,
> "Unable to allocate master resource\n");
> + err = -ENOMEM;
> goto err_master;
> }
> image[i].size_buf = PCI_BUF_SIZE;
>
--
Martyn Welch (Lead Software Engineer) | Registered in England and Wales
GE Intelligent Platforms | (3828642) at 100 Barbirolli Square
T +44(0)1327322748 | Manchester, M2 3AB
E [email protected] | VAT:GB 927559189
On Mon, May 13, 2013 at 09:16:00AM +0100, Martyn Welch wrote:
> On 13/05/13 07:05, Wei Yongjun wrote:
> > From: Wei Yongjun <[email protected]>
> >
> > Fix to return -ENOMEM in the resource alloc error handling
> > case instead of 0, as done elsewhere in this function.
> >
>
> Hi Wei,
>
> Thanks for your patch. As this is resource allocation rather than memory
> allocation that is failing, would -EAGAIN not make more sense than -ENOMEM?
>
ENOMEM is better. EAGAIN is for when trylock() fails etc. In other
words we are not allowed to block and someone is using the lock we
need.
It feels like we discuss error codes a lot on LKML and they should
be documented under Documententation/. The closest thing is
Documentation/i2c/fault-codes.
regards,
dan carpenter
On 13/05/13 09:51, Dan Carpenter wrote:
> On Mon, May 13, 2013 at 09:16:00AM +0100, Martyn Welch wrote:
>> On 13/05/13 07:05, Wei Yongjun wrote:
>>> From: Wei Yongjun <[email protected]>
>>>
>>> Fix to return -ENOMEM in the resource alloc error handling
>>> case instead of 0, as done elsewhere in this function.
>>>
>>
>> Hi Wei,
>>
>> Thanks for your patch. As this is resource allocation rather than memory
>> allocation that is failing, would -EAGAIN not make more sense than -ENOMEM?
>>
>
> ENOMEM is better. EAGAIN is for when trylock() fails etc. In other
> words we are not allowed to block and someone is using the lock we
> need.
>
ENOMEM just doesn't seem to describe the error very well. This error will be
triggered if no free VME windows are available for the driver to use - there
are typically 8 master and 8 slave windows provided in hardware.
How about EBUSY (Device or resource busy)?
> It feels like we discuss error codes a lot on LKML and they should
> be documented under Documententation/. The closest thing is
> Documentation/i2c/fault-codes.
>
I'd been looking at the errno man page since I was under the impression that
these values would typically find their way back to user space.
Martyn
--
Martyn Welch (Lead Software Engineer) | Registered in England and Wales
GE Intelligent Platforms | (3828642) at 100 Barbirolli Square
T +44(0)1327322748 | Manchester, M2 3AB
E [email protected] | VAT:GB 927559189
On Tue, May 14, 2013 at 02:56:17PM +0100, Martyn Welch wrote:
> On 13/05/13 09:51, Dan Carpenter wrote:
> > On Mon, May 13, 2013 at 09:16:00AM +0100, Martyn Welch wrote:
> >> On 13/05/13 07:05, Wei Yongjun wrote:
> >>> From: Wei Yongjun <[email protected]>
> >>>
> >>> Fix to return -ENOMEM in the resource alloc error handling
> >>> case instead of 0, as done elsewhere in this function.
> >>>
> >>
> >> Hi Wei,
> >>
> >> Thanks for your patch. As this is resource allocation rather than memory
> >> allocation that is failing, would -EAGAIN not make more sense than -ENOMEM?
> >>
> >
> > ENOMEM is better. EAGAIN is for when trylock() fails etc. In other
> > words we are not allowed to block and someone is using the lock we
> > need.
> >
>
> ENOMEM just doesn't seem to describe the error very well. This error will be
> triggered if no free VME windows are available for the driver to use - there
> are typically 8 master and 8 slave windows provided in hardware.
>
> How about EBUSY (Device or resource busy)?
EBUSY would work.
regards,
dan carpenter
On 14/05/13 15:19, Dan Carpenter wrote:
> On Tue, May 14, 2013 at 02:56:17PM +0100, Martyn Welch wrote:
>> On 13/05/13 09:51, Dan Carpenter wrote:
>>> On Mon, May 13, 2013 at 09:16:00AM +0100, Martyn Welch wrote:
>>>> On 13/05/13 07:05, Wei Yongjun wrote:
>>>>> From: Wei Yongjun <[email protected]>
>>>>>
>>>>> Fix to return -ENOMEM in the resource alloc error handling
>>>>> case instead of 0, as done elsewhere in this function.
>>>>>
>>>>
>>>> Hi Wei,
>>>>
>>>> Thanks for your patch. As this is resource allocation rather than memory
>>>> allocation that is failing, would -EAGAIN not make more sense than -ENOMEM?
>>>>
>>>
>>> ENOMEM is better. EAGAIN is for when trylock() fails etc. In other
>>> words we are not allowed to block and someone is using the lock we
>>> need.
>>>
>>
>> ENOMEM just doesn't seem to describe the error very well. This error will be
>> triggered if no free VME windows are available for the driver to use - there
>> are typically 8 master and 8 slave windows provided in hardware.
>>
>> How about EBUSY (Device or resource busy)?
>
> EBUSY would work.
>
Hi Wei,
Would you mind resubmitting your patch returning the error code EBUSY rather
than ENOMEM?
Martyn
--
Martyn Welch (Lead Software Engineer) | Registered in England and Wales
GE Intelligent Platforms | (3828642) at 100 Barbirolli Square
T +44(0)1327322748 | Manchester, M2 3AB
E [email protected] | VAT:GB 927559189