[PATCH] hwmon: pmbus: Make reg check and clear faults functions return errors

[Andrew Jeffery]

Some functions exposed by pmbus core conflated errors that occurred when
setting the page to access with errors that occurred when accessing
registers in a page. In some cases, this caused legitimate errors to be
hidden under the guise of the register not being supported.

Signed-off-by: Andrew Jeffery 
---
 Documentation/hwmon/pmbus-core   |  12 ++--
 drivers/hwmon/pmbus/pmbus.h  |   6 +-
 drivers/hwmon/pmbus/pmbus_core.c | 115 +--
 3 files changed, 95 insertions(+), 38 deletions(-)

diff --git a/Documentation/hwmon/pmbus-core b/Documentation/hwmon/pmbus-core
index 8ed10e9ddfb5..3e9f41bb756f 100644
--- a/Documentation/hwmon/pmbus-core
+++ b/Documentation/hwmon/pmbus-core
@@ -218,17 +218,17 @@ Specifically, it provides the following information.
   This function calls the device specific write_byte function if defined.
   Therefore, it must _not_ be called from that function.
 
-  bool pmbus_check_byte_register(struct i2c_client *client, int page, int reg);
+  int pmbus_check_byte_register(struct i2c_client *client, int page, int reg);
 
-  Check if byte register exists. Return true if the register exists, false
-  otherwise.
+  Check if byte register exists. Returns 1 if the register exists, 0 if it does
+  not, and less than zero on an unexpected error.
   This function calls the device specific write_byte function if defined to
   obtain the chip status. Therefore, it must _not_ be called from that 
function.
 
-  bool pmbus_check_word_register(struct i2c_client *client, int page, int reg);
+  int pmbus_check_word_register(struct i2c_client *client, int page, int reg);
 
-  Check if word register exists. Return true if the register exists, false
-  otherwise.
+  Check if word register exists. Returns 1 if the register exists, 0 if it does
+  not, and less than zero on an unexpected error.
   This function calls the device specific write_byte function if defined to
   obtain the chip status. Therefore, it must _not_ be called from that 
function.
 
diff --git a/drivers/hwmon/pmbus/pmbus.h b/drivers/hwmon/pmbus/pmbus.h
index bfcb13bae34b..c53032a04a6f 100644
--- a/drivers/hwmon/pmbus/pmbus.h
+++ b/drivers/hwmon/pmbus/pmbus.h
@@ -413,9 +413,9 @@ int pmbus_write_byte_data(struct i2c_client *client, int 
page, u8 reg,
  u8 value);
 int pmbus_update_byte_data(struct i2c_client *client, int page, u8 reg,
   u8 mask, u8 value);
-void pmbus_clear_faults(struct i2c_client *client);
-bool pmbus_check_byte_register(struct i2c_client *client, int page, int reg);
-bool pmbus_check_word_register(struct i2c_client *client, int page, int reg);
+int pmbus_clear_faults(struct i2c_client *client);
+int pmbus_check_byte_register(struct i2c_client *client, int page, int reg);
+int pmbus_check_word_register(struct i2c_client *client, int page, int reg);
 int pmbus_do_probe(struct i2c_client *client, const struct i2c_device_id *id,
   struct pmbus_driver_info *info);
 int pmbus_do_remove(struct i2c_client *client);
diff --git a/drivers/hwmon/pmbus/pmbus_core.c b/drivers/hwmon/pmbus/pmbus_core.c
index f1eff6b6c798..153700e35431 100644
--- a/drivers/hwmon/pmbus/pmbus_core.c
+++ b/drivers/hwmon/pmbus/pmbus_core.c
@@ -304,18 +304,24 @@ static int _pmbus_read_byte_data(struct i2c_client 
*client, int page, int reg)
return pmbus_read_byte_data(client, page, reg);
 }
 
-static void pmbus_clear_fault_page(struct i2c_client *client, int page)
+static int pmbus_clear_fault_page(struct i2c_client *client, int page)
 {
-   _pmbus_write_byte(client, page, PMBUS_CLEAR_FAULTS);
+   return _pmbus_write_byte(client, page, PMBUS_CLEAR_FAULTS);
 }
 
-void pmbus_clear_faults(struct i2c_client *client)
+int pmbus_clear_faults(struct i2c_client *client)
 {
struct pmbus_data *data = i2c_get_clientdata(client);
+   int rv;
int i;
 
-   for (i = 0; i < data->info->pages; i++)
-   pmbus_clear_fault_page(client, i);
+   for (i = 0; i < data->info->pages; i++) {
+   rv = pmbus_clear_fault_page(client, i);
+   if (rv)
+   return rv;
+   }
+
+   return 0;
 }
 EXPORT_SYMBOL_GPL(pmbus_clear_faults);
 
@@ -333,28 +339,45 @@ static int pmbus_check_status_cml(struct i2c_client 
*client)
return 0;
 }
 
-static bool pmbus_check_register(struct i2c_client *client,
+static int pmbus_check_register(struct i2c_client *client,
 int (*func)(struct i2c_client *client,
 int page, int reg),
 int page, int reg)
 {
+   struct pmbus_data *data;
+   int check;
int rv;
-   struct pmbus_data *data = i2c_get_clientdata(client);
 
-   rv = func(client, page, reg);
-   if (rv >= 0 && !(data->flags & PMBUS_SKIP_STATUS_CHECK))
-   rv = pmbus_check_status_cml(client);
-   pmbus_clear_fault_page(client, -1);
-   ret

Re: [PATCH resend] x86,kvm: Add a kernel parameter to disable PV spinlock

[Juergen Gross]

On 05/09/17 08:58, Peter Zijlstra wrote:
> On Tue, Sep 05, 2017 at 08:28:10AM +0200, Juergen Gross wrote:
>> On 05/09/17 00:21, Davidlohr Bueso wrote:
>>> On Mon, 04 Sep 2017, Peter Zijlstra wrote:
>>>
 For testing its trivial to hack your kernel and I don't feel this is
 something an Admin can make reasonable decisions about.

 So why? In general less knobs is better.
>>>
>>> +1.
>>>
>>> Also, note how b8fa70b51aa (xen, pvticketlocks: Add xen_nopvspin parameter
>>> to disable xen pv ticketlocks) has no justification as to why its wanted
>>> in the first place. The only thing I could find was from 15a3eac0784
>>> (xen/spinlock: Document the xen_nopvspin parameter):
>>>
>>> "Useful for diagnosing issues and comparing benchmarks in over-commit
>>> CPU scenarios."
>>
>> Hmm, I think I should clarify the Xen knob, as I was the one requesting
>> it:
>>
>> In my previous employment we had a configuration where dom0 ran
>> exclusively on a dedicated set of physical cpus. We experienced
>> scalability problems when doing I/O performance tests: with a decent
>> number of dom0 cpus we achieved throughput of 700 MB/s with only 20%
>> cpu load in dom0. A higher dom0 cpu count let the throughput drop to
>> about 150 MB/s and cpu load was up to 100%. Reason was the additional
>> load due to hypervisor interactions on a high frequency lock.
>>
>> So in special configurations at least for Xen the knob is useful for
>> production environment.
> 
> So the problem with qspinlock is that it will revert to a classic
> test-and-set spinlock if you don't do paravirt but are running a HV.

In the Xen case we just use the bare metal settings when xen_nopvspin
has been specified. So paravirt, but without modifying any pv_lock_ops
functions.


Juergen

> 
> And test-and-set is unfair and has all kinds of ugly starvation cases,
> esp on slightly bigger hardware.
> 
> So if we'd want to cater to the 1:1 virt case, we'll need to come up
> with something else. _IF_ it is an issue of course.
> 

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

Re: [PATCH resend] x86,kvm: Add a kernel parameter to disable PV spinlock

[Peter Zijlstra]

On Tue, Sep 05, 2017 at 08:57:16AM +0200, Oscar Salvador wrote:
> It may be that the original patch was just to keep consistency between Xen
> and KVM, and also only for testing purposes.
> But we find a case when a customer of ours is running some workloads with
> 1<->1 mapping between physical cores and virtual cores, and we realized that
> with the pv spinlocks disabled there is a 4-5% of performance gain.

There are very definite downsides to using a test-and-set spinlock.

A much better option would be one that forces the use of native
qspinlock in the 1:1 case. That means you have to fail both pv_enabled()
and virt_spin_lock().

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

Re: [PATCH resend] x86,kvm: Add a kernel parameter to disable PV spinlock

[Peter Zijlstra]

On Tue, Sep 05, 2017 at 09:35:40AM +0200, Juergen Gross wrote:
> > So the problem with qspinlock is that it will revert to a classic
> > test-and-set spinlock if you don't do paravirt but are running a HV.
> 
> In the Xen case we just use the bare metal settings when xen_nopvspin
> has been specified. So paravirt, but without modifying any pv_lock_ops
> functions.

See arch/x86/include/asm/qspinlock.h:virt_spin_lock(). Unless you clear
X86_FEATURE_HYPERVISOR you get a test-and-set spinlock.

And as the comment there says, this is a fallback for !paravirt enabled
hypervisors to avoid the worst of the lock holder preemption crud.

But this very much does not deal with the 1:1 case nicely.
--
To unsubscribe from this list: send the line "unsubscribe linux-doc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH resend] x86,kvm: Add a kernel parameter to disable PV spinlock

[Juergen Gross]

On 05/09/17 10:10, Peter Zijlstra wrote:
> On Tue, Sep 05, 2017 at 09:35:40AM +0200, Juergen Gross wrote:
>>> So the problem with qspinlock is that it will revert to a classic
>>> test-and-set spinlock if you don't do paravirt but are running a HV.
>>
>> In the Xen case we just use the bare metal settings when xen_nopvspin
>> has been specified. So paravirt, but without modifying any pv_lock_ops
>> functions.
> 
> See arch/x86/include/asm/qspinlock.h:virt_spin_lock(). Unless you clear
> X86_FEATURE_HYPERVISOR you get a test-and-set spinlock.
> 
> And as the comment there says, this is a fallback for !paravirt enabled
> hypervisors to avoid the worst of the lock holder preemption crud.
> 
> But this very much does not deal with the 1:1 case nicely.
> 

Aah, now I've got it.

So maybe we should add virt_spin_lock() to pv_lock_ops? This way e.g.
xen_nopvspin could tweak just the virt_spin_lock() case by letting it
return false all the time?

In case you agree I can setup a patch...


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

Re: [PATCH resend] x86,kvm: Add a kernel parameter to disable PV spinlock

[Peter Zijlstra]

On Tue, Sep 05, 2017 at 10:14:21AM +0200, Juergen Gross wrote:
> On 05/09/17 10:10, Peter Zijlstra wrote:
> > On Tue, Sep 05, 2017 at 09:35:40AM +0200, Juergen Gross wrote:
> >>> So the problem with qspinlock is that it will revert to a classic
> >>> test-and-set spinlock if you don't do paravirt but are running a HV.
> >>
> >> In the Xen case we just use the bare metal settings when xen_nopvspin
> >> has been specified. So paravirt, but without modifying any pv_lock_ops
> >> functions.
> > 
> > See arch/x86/include/asm/qspinlock.h:virt_spin_lock(). Unless you clear
> > X86_FEATURE_HYPERVISOR you get a test-and-set spinlock.
> > 
> > And as the comment there says, this is a fallback for !paravirt enabled
> > hypervisors to avoid the worst of the lock holder preemption crud.
> > 
> > But this very much does not deal with the 1:1 case nicely.
> > 
> 
> Aah, now I've got it.
> 
> So maybe we should add virt_spin_lock() to pv_lock_ops? This way e.g.
> xen_nopvspin could tweak just the virt_spin_lock() case by letting it
> return false all the time?

Hmm, that might work. Could we somehow nop that call when
!X86_FEATURE_HYPERVISOR?, that saves native from having to do the call
and would be a win for everyone.
--
To unsubscribe from this list: send the line "unsubscribe linux-doc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH resend] x86,kvm: Add a kernel parameter to disable PV spinlock

[Juergen Gross]

On 05/09/17 10:28, Peter Zijlstra wrote:
> On Tue, Sep 05, 2017 at 10:14:21AM +0200, Juergen Gross wrote:
>> On 05/09/17 10:10, Peter Zijlstra wrote:
>>> On Tue, Sep 05, 2017 at 09:35:40AM +0200, Juergen Gross wrote:
> So the problem with qspinlock is that it will revert to a classic
> test-and-set spinlock if you don't do paravirt but are running a HV.

 In the Xen case we just use the bare metal settings when xen_nopvspin
 has been specified. So paravirt, but without modifying any pv_lock_ops
 functions.
>>>
>>> See arch/x86/include/asm/qspinlock.h:virt_spin_lock(). Unless you clear
>>> X86_FEATURE_HYPERVISOR you get a test-and-set spinlock.
>>>
>>> And as the comment there says, this is a fallback for !paravirt enabled
>>> hypervisors to avoid the worst of the lock holder preemption crud.
>>>
>>> But this very much does not deal with the 1:1 case nicely.
>>>
>>
>> Aah, now I've got it.
>>
>> So maybe we should add virt_spin_lock() to pv_lock_ops? This way e.g.
>> xen_nopvspin could tweak just the virt_spin_lock() case by letting it
>> return false all the time?
> 
> Hmm, that might work. Could we somehow nop that call when
> !X86_FEATURE_HYPERVISOR?, that saves native from having to do the call
> and would be a win for everyone.

So in fact we want a "always false" shortcut for bare metal and for any
virtualization environment selecting bare metal behavior.

I'll have a try.


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

Re: [PATCH resend] x86,kvm: Add a kernel parameter to disable PV spinlock

[Peter Zijlstra]

On Tue, Sep 05, 2017 at 10:52:06AM +0200, Juergen Gross wrote:
> > Hmm, that might work. Could we somehow nop that call when
> > !X86_FEATURE_HYPERVISOR?, that saves native from having to do the call
> > and would be a win for everyone.
> 
> So in fact we want a "always false" shortcut for bare metal and for any
> virtualization environment selecting bare metal behavior.
> 
> I'll have a try.

Right, so for native I think you can do this:


diff --git a/arch/x86/kernel/paravirt_patch_64.c 
b/arch/x86/kernel/paravirt_patch_64.c
index 11aaf1eaa0e4..4e9839001291 100644
--- a/arch/x86/kernel/paravirt_patch_64.c
+++ b/arch/x86/kernel/paravirt_patch_64.c
@@ -21,6 +21,7 @@ DEF_NATIVE(, mov64, "mov %rdi, %rax");
 #if defined(CONFIG_PARAVIRT_SPINLOCKS)
 DEF_NATIVE(pv_lock_ops, queued_spin_unlock, "movb $0, (%rdi)");
 DEF_NATIVE(pv_lock_ops, vcpu_is_preempted, "xor %rax, %rax");
+DEF_NATIVE(pv_lock_ops, virt_spin_lock, "xor %rax, %rax");
 #endif
 
 unsigned paravirt_patch_ident_32(void *insnbuf, unsigned len)
@@ -77,6 +78,13 @@ unsigned native_patch(u8 type, u16 clobbers, void *ibuf,
goto patch_site;
}
goto patch_default;
+   case PARAVIRT_PATCH(pv_lock_ops.virt_spin_lock):
+   if (!this_cpu_has(X86_FEATURE_HYPERVISOR)) {
+   start = start_pv_lock_ops_virt_spin_lock;
+   end   = end_pv_lock_ops_virt_spin_lock;
+   goto patch_side;
+   }
+   goto patch_default;
 #endif
 
default:
--
To unsubscribe from this list: send the line "unsubscribe linux-doc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH resend] x86,kvm: Add a kernel parameter to disable PV spinlock

[Juergen Gross]

On 05/09/17 11:03, Peter Zijlstra wrote:
> On Tue, Sep 05, 2017 at 10:52:06AM +0200, Juergen Gross wrote:
>>> Hmm, that might work. Could we somehow nop that call when
>>> !X86_FEATURE_HYPERVISOR?, that saves native from having to do the call
>>> and would be a win for everyone.
>>
>> So in fact we want a "always false" shortcut for bare metal and for any
>> virtualization environment selecting bare metal behavior.
>>
>> I'll have a try.
> 
> Right, so for native I think you can do this:
> 
> 
> diff --git a/arch/x86/kernel/paravirt_patch_64.c 
> b/arch/x86/kernel/paravirt_patch_64.c
> index 11aaf1eaa0e4..4e9839001291 100644
> --- a/arch/x86/kernel/paravirt_patch_64.c
> +++ b/arch/x86/kernel/paravirt_patch_64.c
> @@ -21,6 +21,7 @@ DEF_NATIVE(, mov64, "mov %rdi, %rax");
>  #if defined(CONFIG_PARAVIRT_SPINLOCKS)
>  DEF_NATIVE(pv_lock_ops, queued_spin_unlock, "movb $0, (%rdi)");
>  DEF_NATIVE(pv_lock_ops, vcpu_is_preempted, "xor %rax, %rax");
> +DEF_NATIVE(pv_lock_ops, virt_spin_lock, "xor %rax, %rax");
>  #endif
>  
>  unsigned paravirt_patch_ident_32(void *insnbuf, unsigned len)
> @@ -77,6 +78,13 @@ unsigned native_patch(u8 type, u16 clobbers, void *ibuf,
>   goto patch_site;
>   }
>   goto patch_default;
> + case PARAVIRT_PATCH(pv_lock_ops.virt_spin_lock):
> + if (!this_cpu_has(X86_FEATURE_HYPERVISOR)) {
> + start = start_pv_lock_ops_virt_spin_lock;
> + end   = end_pv_lock_ops_virt_spin_lock;
> + goto patch_side;
> + }
> + goto patch_default;

I'd rather add a test to paravirt_patch_default() similar to the
_paravirt_ident_* cases for catching the Xen (and possibly KVM) cases,
too.

I just need to add _paravirt_zero_[32|64] functions I can use for bare
metal and the nopvspin cases.


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

Re: [PATCH 1/2] docs-rst: media: Don't use \small for V4L2_PIX_FMT_SRGGB10 documentation

[Sakari Ailus]

Hi Mauro,

On Mon, Sep 04, 2017 at 05:41:27PM -0300, Mauro Carvalho Chehab wrote:
> From: Sakari Ailus 
> 
> There appears to be an issue in using \small in certain cases on Sphinx
> 1.4 and 1.5. Other format documents don't use \small either, remove it
> from here as well.
> 
> [mche...@s-opensource.com: kept tabularcolumns - readjusted - and
>  add a few blank lines for it to display better]
> Signed-off-by: Sakari Ailus 
> Signed-off-by: Mauro Carvalho Chehab 

Thanks!

For both:

Acked-by: Sakari Ailus 

-- 
Sakari Ailus
e-mail: sakari.ai...@iki.fi
--
To unsubscribe from this list: send the line "unsubscribe linux-doc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 1/2] media: dvb uapi: move frontend legacy API to another part of the book

[Mauro Carvalho Chehab]

There's a chapter for the legacy APIs. Move the frontend DVBv3
API to it, and update the chapter's introduction accordingly.

Signed-off-by: Mauro Carvalho Chehab 
---
 Documentation/media/uapi/dvb/frontend.rst|  1 -
 Documentation/media/uapi/dvb/legacy_dvb_apis.rst | 21 ++---
 2 files changed, 14 insertions(+), 8 deletions(-)

diff --git a/Documentation/media/uapi/dvb/frontend.rst 
b/Documentation/media/uapi/dvb/frontend.rst
index b9cfcad39823..4967c48d46ce 100644
--- a/Documentation/media/uapi/dvb/frontend.rst
+++ b/Documentation/media/uapi/dvb/frontend.rst
@@ -54,4 +54,3 @@ Data types and ioctl definitions can be accessed by including
 dvb-fe-read-status
 dvbproperty
 frontend_fcalls
-frontend_legacy_dvbv3_api
diff --git a/Documentation/media/uapi/dvb/legacy_dvb_apis.rst 
b/Documentation/media/uapi/dvb/legacy_dvb_apis.rst
index 7eb14d6f729f..e1b2c9c7b620 100644
--- a/Documentation/media/uapi/dvb/legacy_dvb_apis.rst
+++ b/Documentation/media/uapi/dvb/legacy_dvb_apis.rst
@@ -6,20 +6,27 @@
 Digital TV Deprecated APIs
 ***
 
-The APIs described here are kept only for historical reasons. There's
-just one driver for a very legacy hardware that uses this API. No modern
-drivers should use it. Instead, audio and video should be using the V4L2
-and ALSA APIs, and the pipelines should be set using the Media
-Controller API
+The APIs described here **should not** be used on new drivers or applications.
 
-.. note::
+The DVBv3 frontend API has issues with new delivery systems, including
+DVB-S2, DVB-T2, ISDB, etc.
+
+There's just one driver for a very legacy hardware using the Digital TV
+audio and video APIs. No modern drivers should use it. Instead, audio and
+video should be using the V4L2 and ALSA APIs, and the pipelines should
+be set via the Media Controller API.
+
+.. attention::
 
The APIs described here doesn't necessarily reflect the current
-   code implementation.
+   code implementation, as this section of the document was written
+   for DVB version 1, while the code reflects DVB version 3
+   implementation.
 
 
 .. toctree::
 :maxdepth: 1
 
+frontend_legacy_dvbv3_api
 video
 audio
-- 
2.13.5

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

[PATCH 2/2] media: dvb headers: make checkpatch happier

[Mauro Carvalho Chehab]

Adjust dvb ca.h, dmx.h and frontend.h in order to make
checkpatch happier. Now, it only complains about the typedefs,
and those are there just to provide backward userspace
compatibility.

Signed-off-by: Mauro Carvalho Chehab 
---
 include/uapi/linux/dvb/ca.h   | 2 +-
 include/uapi/linux/dvb/dmx.h  | 5 ++---
 include/uapi/linux/dvb/frontend.h | 6 +++---
 3 files changed, 6 insertions(+), 7 deletions(-)

diff --git a/include/uapi/linux/dvb/ca.h b/include/uapi/linux/dvb/ca.h
index 24fc38efbc2b..cb150029fdff 100644
--- a/include/uapi/linux/dvb/ca.h
+++ b/include/uapi/linux/dvb/ca.h
@@ -139,7 +139,7 @@ struct ca_descr {
 #define CA_SEND_MSG   _IOW('o', 133, struct ca_msg)
 #define CA_SET_DESCR  _IOW('o', 134, struct ca_descr)
 
-#if !defined (__KERNEL__)
+#if !defined(__KERNEL__)
 
 /* This is needed for legacy userspace support */
 typedef struct ca_slot_info ca_slot_info_t;
diff --git a/include/uapi/linux/dvb/dmx.h b/include/uapi/linux/dvb/dmx.h
index 4e3f3a2fe83f..4aa5f6a1815a 100644
--- a/include/uapi/linux/dvb/dmx.h
+++ b/include/uapi/linux/dvb/dmx.h
@@ -189,8 +189,7 @@ struct dmx_sct_filter_params {
  * @pes_type:  Type of the pes filter, as specified by &enum dmx_pes_type.
  * @flags: Demux PES flags.
  */
-struct dmx_pes_filter_params
-{
+struct dmx_pes_filter_params {
__u16   pid;
enum dmx_input  input;
enum dmx_output output;
@@ -221,7 +220,7 @@ struct dmx_stc {
 #define DMX_ADD_PID  _IOW('o', 51, __u16)
 #define DMX_REMOVE_PID   _IOW('o', 52, __u16)
 
-#if !defined (__KERNEL__)
+#if !defined(__KERNEL__)
 
 /* This is needed for legacy userspace support */
 typedef enum dmx_output dmx_output_t;
diff --git a/include/uapi/linux/dvb/frontend.h 
b/include/uapi/linux/dvb/frontend.h
index fc2edb6014fe..861cacd5711f 100644
--- a/include/uapi/linux/dvb/frontend.h
+++ b/include/uapi/linux/dvb/frontend.h
@@ -907,7 +907,7 @@ struct dtv_properties {
 #define FE_SET_PROPERTY   _IOW('o', 82, struct dtv_properties)
 #define FE_GET_PROPERTY   _IOR('o', 83, struct dtv_properties)
 
-#if defined(__DVB_CORE__) || !defined (__KERNEL__)
+#if defined(__DVB_CORE__) || !defined(__KERNEL__)
 
 /*
  * DEPRECATED: Everything below is deprecated in favor of DVBv5 API
@@ -982,8 +982,8 @@ struct dvb_ofdm_parameters {
 };
 
 struct dvb_frontend_parameters {
-   __u32 frequency; /* (absolute) frequency in Hz for DVB-C/DVB-T/ATSC 
*/
-/* intermediate frequency in kHz for DVB-S */
+   __u32 frequency;  /* (absolute) frequency in Hz for DVB-C/DVB-T/ATSC */
+ /* intermediate frequency in kHz for DVB-S */
fe_spectral_inversion_t inversion;
union {
struct dvb_qpsk_parameters qpsk;/* DVB-S */
-- 
2.13.5

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

[PATCH] media: add qcom_camss.rst to v4l-drivers rst file

[Mauro Carvalho Chehab]

Avoid this warning:
/devel/v4l/docs/Documentation/media/v4l-drivers/qcom_camss.rst:: 
WARNING: document isn't included in any toctree

Signed-off-by: Mauro Carvalho Chehab 
---
 Documentation/media/v4l-drivers/index.rst | 1 +
 1 file changed, 1 insertion(+)

diff --git a/Documentation/media/v4l-drivers/index.rst 
b/Documentation/media/v4l-drivers/index.rst
index 3643e63c4e46..679238e786a7 100644
--- a/Documentation/media/v4l-drivers/index.rst
+++ b/Documentation/media/v4l-drivers/index.rst
@@ -52,6 +52,7 @@ For more details see the file COPYING in the source 
distribution of Linux.
philips
pvrusb2
pxa_camera
+   qcom_camss
radiotrack
rcar-fdp1
saa7134
-- 
2.13.5

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

Re: [PATCH v7 00/19] simplify crypto wait for async op

[Harsh Jain]

On Sun, Sep 3, 2017 at 11:47 AM, Gilad Ben-Yossef  wrote:
> On Thu, Aug 31, 2017 at 3:31 PM, Harsh Jain  wrote:
>> HI Gilad,
>>
>> I think we need an update in ESP also. Now EBUSY return means driver
>> has accepted, Packet should not be dropped in
>>
>> esp_output_tail() function.
>
> Good catch. You are right and the same holds true for ah_output() in ah4.c.
>
> But I do wonder, the code there now treats -EBUSY as a special case
> and returns NET_XMIT_DROP
> but if an AEAD or AHASH transformation return some other error, like
> -ENOMEM or -EINVAL shouldn't
> we return NET_XMIT_DROP in that case too?
I think we should not,  XMIT_DROP implies drop current packet only,
later on when device is recovered from busy state, Upper layer
protocol(TCP) will re-transmit the packet. It helps in flow control.
>
> Any ideas?
>
> Gilad
>
>>
>>
>> On Thu, Aug 24, 2017 at 7:48 PM, Gilad Ben-Yossef  
>> wrote:
>>> Many users of kernel async. crypto services have a pattern of
>>> starting an async. crypto op and than using a completion
>>> to wait for it to end.
>>>
>>> This patch set simplifies this common use case in two ways:
>>>
>>> First, by separating the return codes of the case where a
>>> request is queued to a backlog due to the provider being
>>> busy (-EBUSY) from the case the request has failed due
>>> to the provider being busy and backlogging is not enabled
>>> (-EAGAIN).
>>>
>>> Next, this change is than built on to create a generic API
>>> to wait for a async. crypto operation to complete.
>>>
>>> The end result is a smaller code base and an API that is
>>> easier to use and more difficult to get wrong.
>>>
>>> The patch set was boot tested on x86_64 and arm64 which
>>> at the very least tests the crypto users via testmgr and
>>> tcrypt but I do note that I do not have access to some
>>> of the HW whose drivers are modified nor do I claim I was
>>> able to test all of the corner cases.
>>>
>>> The patch set is based upon linux-next release tagged
>>> next-20170824.
>>>
>>> Changes from v6:
>>> - Fix brown paper bag compile error on marvell/cesa
>>>   code.
>>>
>>> Changes from v5:
>>> - Remove redundant new line as spotted by Jonathan
>>>   Cameron.
>>> - Reworded dm-verity change commit message to better
>>>   clarify potential issue averted by change as
>>>   pointed out by Mikulas Patocka.
>>>
>>> Changes from v4:
>>> - Rebase on top of latest algif changes from Stephan
>>>   Mueller.
>>> - Fix typo in ccp patch title.
>>>
>>> Changes from v3:
>>> - Instead of changing the return code to indicate
>>>   backlog queueing, change the return code to indicate
>>>   transient busy state, as suggested by Herbert Xu.
>>>
>>> Changes from v2:
>>> - Patch title changed from "introduce crypto wait for
>>>   async op" to better reflect the current state.
>>> - Rebase on top of latest linux-next.
>>> - Add a new return code of -EIOCBQUEUED for backlog
>>>   queueing, as suggested by Herbert Xu.
>>> - Transform more users to the new API.
>>> - Update the drbg change to account for new init as
>>>   indicated by Stephan Muller.
>>>
>>> Changes from v1:
>>> - Address review comments from Eric Biggers.
>>> - Separated out bug fixes of existing code and rebase
>>>   on top of that patch set.
>>> - Rename 'ecr' to 'wait' in fscrypto code.
>>> - Split patch introducing the new API from the change
>>>   moving over the algif code which it originated from
>>>   to the new API.
>>> - Inline crypto_wait_req().
>>> - Some code indentation fixes.
>>>
>>> Gilad Ben-Yossef (19):
>>>   crypto: change transient busy return code to -EAGAIN
>>>   crypto: ccp: use -EAGAIN for transient busy indication
>>>   crypto: remove redundant backlog checks on EBUSY
>>>   crypto: marvell/cesa: remove redundant backlog checks on EBUSY
>>>   crypto: introduce crypto wait for async op
>>>   crypto: move algif to generic async completion
>>>   crypto: move pub key to generic async completion
>>>   crypto: move drbg to generic async completion
>>>   crypto: move gcm to generic async completion
>>>   crypto: move testmgr to generic async completion
>>>   fscrypt: move to generic async completion
>>>   dm: move dm-verity to generic async completion
>>>   cifs: move to generic async completion
>>>   ima: move to generic async completion
>>>   crypto: tcrypt: move to generic async completion
>>>   crypto: talitos: move to generic async completion
>>>   crypto: qce: move to generic async completion
>>>   crypto: mediatek: move to generic async completion
>>>   crypto: adapt api sample to use async. op wait
>>>
>>>  Documentation/crypto/api-samples.rst |  52 ++---
>>>  crypto/af_alg.c  |  27 -
>>>  crypto/ahash.c   |  12 +--
>>>  crypto/algapi.c  |   6 +-
>>>  crypto/algif_aead.c  |   8 +-
>>>  crypto/algif_hash.c  |  50 +
>>>  crypto/algif_skcipher.c  |   9 +-
>>>  crypto/api.c |  13 +++
>>>  crypto/asymmetric_keys/

[PATCH 3/3 v11] printk: Add monotonic, boottime, and realtime timestamps

[Prarit Bhargava]

printk.time=1/CONFIG_PRINTK_TIME=1 adds a unmodified local hardware clock
timestamp to printk messages.  The local hardware clock loses time each
day making it difficult to determine exactly when an issue has occurred in
the kernel log, and making it difficult to determine how kernel and
hardware issues relate to each other in real time.

Make printk output different timestamps by adding options for no
timestamp, the local hardware clock, the monotonic clock, the boottime
clock, and the real clock.  Allow a user to pick one of the clocks by
using the printk.time kernel parameter.  Output the type of clock in
/sys/module/printk/parameters/time so userspace programs can interpret the
timestamp.

v2: Use peterz's suggested Kconfig options.  Merge patchset together.
Fix i386 !CONFIG_PRINTK builds.

v3: Fixed x86_64_defconfig. Added printk_time_type enum and
printk_time_str for better output. Added BOOTTIME clock functionality.

v4: Fix messages, add additional printk.time options, and fix configs.

v5: Renaming of structures, and allow printk_time_set() to
evaluate substrings of entries (eg: allow 'r', 'real', 'realtime').  From
peterz, make fast functions return 0 until timekeeping is initialized
(removes timekeeping_active & ktime_get_boot|real_log_ts() suggested by
 tglx and adds ktime_get_real_offset()).  Switch to a function pointer
for printk_get_ts() and reference fast functions.  Make timestamp_sources enum
match choice options for CONFIG_PRINTK_TIME (adds PRINTK_TIME_UNDEFINED).

v6: Define PRINTK_TIME_UNDEFINED for !CONFIG_PRINTK builds.  Separate
timekeeping changes into separate patch.  Minor include file cleanup.

v7: Add default case to printk_set_timestamp() and add PRINTK_TIME_DEBUG
for users that want to set timestamp to different values during runtime.
Add jstultz' Kconfig to avoid defconfig churn.

v8: Add CONFIG_PRINTK_TIME_DEBUG to allow timestamp runtime switching.
Rename PRINTK_TIME_DISABLE to PRINTK_TIME_DISABLED.  Rename
printk_set_timestamp() to printk_set_ts_func().  Separate
printk_set_ts_func() and printk_get_first_ts() portions.  Rename param
functions.  Adjust configs, enum, and timestamp_sources_str to be 0-4.
Add mention realtime clock is UTC in Documentation.

v9: Fix typo.  Add __ktime_get_real_fast_ns_unsafe().

v10: Remove time parameter restrictions.

Signed-off-by: Prarit Bhargava 
Cc: Mark Salyzyn 
Cc: Jonathan Corbet 
Cc: Petr Mladek 
Cc: Sergey Senozhatsky 
Cc: Steven Rostedt 
Cc: John Stultz 
Cc: Thomas Gleixner 
Cc: Stephen Boyd 
Cc: Andrew Morton 
Cc: Greg Kroah-Hartman 
Cc: "Paul E. McKenney" 
Cc: Christoffer Dall 
Cc: Deepa Dinamani 
Cc: Ingo Molnar 
Cc: Joel Fernandes 
Cc: Prarit Bhargava 
Cc: Kees Cook 
Cc: Peter Zijlstra 
Cc: Geert Uytterhoeven 
Cc: "Luis R. Rodriguez" 
Cc: Nicholas Piggin 
Cc: "Jason A. Donenfeld" 
Cc: Olof Johansson 
Cc: Josh Poimboeuf 
Cc: linux-doc@vger.kernel.org
[jstultz: reworked Kconfig settings to avoid defconfig noise]
Signed-off-by: John Stultz 
---
 Documentation/admin-guide/kernel-parameters.txt |   6 +-
 kernel/printk/printk.c  | 113 ++--
 lib/Kconfig.debug   |  48 +-
 3 files changed, 159 insertions(+), 8 deletions(-)

diff --git a/Documentation/admin-guide/kernel-parameters.txt 
b/Documentation/admin-guide/kernel-parameters.txt
index d9c171ce4190..8d6b194533af 100644
--- a/Documentation/admin-guide/kernel-parameters.txt
+++ b/Documentation/admin-guide/kernel-parameters.txt
@@ -3188,8 +3188,10 @@
ratelimit - ratelimit the logging
Default: ratelimit
 
-   printk.time=Show timing data prefixed to each printk message line
-   Format:   (1/Y/y=enable, 0/N/n=disable)
+   printk.time=Show timestamp prefixed to each printk message line
+   Format: 
+   (0/N/n/disable, 1/Y/y/local,
+b/boot, m/monotonic, r/realtime (in UTC))
 
processor.max_cstate=   [HW,ACPI]
Limit processor to maximum C-state
diff --git a/kernel/printk/printk.c b/kernel/printk/printk.c
index fc47863f629c..5aaeb1ebd26c 100644
--- a/kernel/printk/printk.c
+++ b/kernel/printk/printk.c
@@ -576,6 +576,9 @@ static u32 truncate_msg(u16 *text_len, u16 *trunc_msg_len,
return msg_used_size(*text_len + *trunc_msg_len, 0, pad_len);
 }
 
+static u64 printk_get_first_ts(void);
+static u64 (*printk_get_ts)(void) = printk_get_first_ts;
+
 /* insert record into the buffer, discard old ones, update heads */
 static int log_store(int facility, int level,
 enum log_flags flags, u64 ts_nsec,
@@ -624,7 +627,7 @@ static int log_store(int facility, int level,
if (ts_nsec > 0)
msg->ts_nsec = ts_nsec;
else
-   msg->ts_nsec = local_clock();
+   msg->ts_nsec = printk_get_ts();
memset(log_dict(msg) + dict_len, 0, pad_len);
msg->l

[PATCH 0/3 v11] printk: Add new timestamps

[Prarit Bhargava]

printk.time=1/CONFIG_PRINTK_TIME=1 adds a unmodified local hardware clock
timestamp to printk messages.  The local hardware clock loses time each
day making it difficult to determine exactly when an issue has occurred in
the kernel log, and making it difficult to determine how kernel and
hardware issues relate to each other in real time.

Add monotonic, boottime, and real clock timestamps in addition to the existing
local hardware clock timestamp.

Signed-off-by: Prarit Bhargava 
Cc: Mark Salyzyn 
Cc: Jonathan Corbet 
Cc: Petr Mladek 
Cc: Sergey Senozhatsky 
Cc: Steven Rostedt 
Cc: John Stultz 
Cc: Thomas Gleixner 
Cc: Stephen Boyd 
Cc: Andrew Morton 
Cc: Greg Kroah-Hartman 
Cc: "Paul E. McKenney" 
Cc: Christoffer Dall 
Cc: Deepa Dinamani 
Cc: Ingo Molnar 
Cc: Joel Fernandes 
Cc: Prarit Bhargava 
Cc: Kees Cook 
Cc: Peter Zijlstra 
Cc: Geert Uytterhoeven 
Cc: "Luis R. Rodriguez" 
Cc: Nicholas Piggin 
Cc: "Jason A. Donenfeld" 
Cc: Olof Johansson 
Cc: Josh Poimboeuf 
Cc: linux-doc@vger.kernel.org
[jstultz: reworked Kconfig settings to avoid defconfig noise]
Signed-off-by: John Stultz 

Prarit Bhargava (2):
  time: Make fast functions return 0 before timekeeping is initialized
  printk: Add monotonic, boottime, and realtime timestamps

Thomas Gleixner (1):
  timekeeping: Provide NMI safe access to clock realtime

 Documentation/admin-guide/kernel-parameters.txt |   6 +-
 include/linux/timekeeper_internal.h |   6 +-
 include/linux/timekeeping.h |   1 +
 kernel/printk/printk.c  | 113 ++--
 kernel/time/timekeeping.c   |  72 +--
 lib/Kconfig.debug   |  48 +-
 6 files changed, 228 insertions(+), 18 deletions(-)

-- 
1.8.5.5

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

Re: [PATCH v7 00/19] simplify crypto wait for async op

[Gilad Ben-Yossef]

On Tue, Sep 5, 2017 at 2:23 PM, Harsh Jain  wrote:
> On Sun, Sep 3, 2017 at 11:47 AM, Gilad Ben-Yossef  wrote:
>> On Thu, Aug 31, 2017 at 3:31 PM, Harsh Jain  wrote:
>>> HI Gilad,
>>>
>>> I think we need an update in ESP also. Now EBUSY return means driver
>>> has accepted, Packet should not be dropped in
>>>
>>> esp_output_tail() function.
>>
>> Good catch. You are right and the same holds true for ah_output() in ah4.c.
>>
>> But I do wonder, the code there now treats -EBUSY as a special case
>> and returns NET_XMIT_DROP
>> but if an AEAD or AHASH transformation return some other error, like
>> -ENOMEM or -EINVAL shouldn't
>> we return NET_XMIT_DROP in that case too?
> I think we should not,  XMIT_DROP implies drop current packet only,
> later on when device is recovered from busy state, Upper layer
> protocol(TCP) will re-transmit the packet. It helps in flow control.
>>

I see. Makes sense.

Thanks,
Gilad

-- 
Gilad Ben-Yossef
Chief Coffee Drinker

"If you take a class in large-scale robotics, can you end up in a
situation where the homework eats your dog?"
 -- Jean-Baptiste Queru
--
To unsubscribe from this list: send the line "unsubscribe linux-doc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH v8 02/20] crypto: ccp: use -EAGAIN for transient busy indication

[Gilad Ben-Yossef]

Replace -EBUSY with -EAGAIN when reporting transient busy
indication in the absence of backlog.

Signed-off-by: Gilad Ben-Yossef 
Reviewed-by: Gary R Hook 

---

Please squash this patch with the previous one when merging upstream.
---
 drivers/crypto/ccp/ccp-crypto-main.c | 8 +++-
 drivers/crypto/ccp/ccp-dev.c | 7 +--
 2 files changed, 8 insertions(+), 7 deletions(-)

diff --git a/drivers/crypto/ccp/ccp-crypto-main.c 
b/drivers/crypto/ccp/ccp-crypto-main.c
index 35a9de7..403ff0a 100644
--- a/drivers/crypto/ccp/ccp-crypto-main.c
+++ b/drivers/crypto/ccp/ccp-crypto-main.c
@@ -222,9 +222,10 @@ static int ccp_crypto_enqueue_cmd(struct ccp_crypto_cmd 
*crypto_cmd)
 
/* Check if the cmd can/should be queued */
if (req_queue.cmd_count >= CCP_CRYPTO_MAX_QLEN) {
-   ret = -EBUSY;
-   if (!(crypto_cmd->cmd->flags & CCP_CMD_MAY_BACKLOG))
+   if (!(crypto_cmd->cmd->flags & CCP_CMD_MAY_BACKLOG)) {
+   ret = -EAGAIN;
goto e_lock;
+   }
}
 
/* Look for an entry with the same tfm.  If there is a cmd
@@ -243,9 +244,6 @@ static int ccp_crypto_enqueue_cmd(struct ccp_crypto_cmd 
*crypto_cmd)
ret = ccp_enqueue_cmd(crypto_cmd->cmd);
if (!ccp_crypto_success(ret))
goto e_lock;/* Error, don't queue it */
-   if ((ret == -EBUSY) &&
-   !(crypto_cmd->cmd->flags & CCP_CMD_MAY_BACKLOG))
-   goto e_lock;/* Not backlogging, don't queue it */
}
 
if (req_queue.cmd_count >= CCP_CRYPTO_MAX_QLEN) {
diff --git a/drivers/crypto/ccp/ccp-dev.c b/drivers/crypto/ccp/ccp-dev.c
index 4e029b1..3d637e3 100644
--- a/drivers/crypto/ccp/ccp-dev.c
+++ b/drivers/crypto/ccp/ccp-dev.c
@@ -292,9 +292,12 @@ int ccp_enqueue_cmd(struct ccp_cmd *cmd)
i = ccp->cmd_q_count;
 
if (ccp->cmd_count >= MAX_CMD_QLEN) {
-   ret = -EBUSY;
-   if (cmd->flags & CCP_CMD_MAY_BACKLOG)
+   if (cmd->flags & CCP_CMD_MAY_BACKLOG) {
+   ret = -EBUSY;
list_add_tail(&cmd->entry, &ccp->backlog);
+   } else {
+   ret = -EAGAIN;
+   }
} else {
ret = -EINPROGRESS;
ccp->cmd_count++;
-- 
2.1.4

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

[PATCH v8 00/20] simplify crypto wait for async op

[Gilad Ben-Yossef]

Many users of kernel async. crypto services have a pattern of
starting an async. crypto op and than using a completion
to wait for it to end.

This patch set simplifies this common use case in two ways:

First, by separating the return codes of the case where a
request is queued to a backlog due to the provider being
busy (-EBUSY) from the case the request has failed due
to the provider being busy and backlogging is not enabled
(-EAGAIN).

Next, this change is than built on to create a generic API
to wait for a async. crypto operation to complete.

The end result is a smaller code base and an API that is
easier to use and more difficult to get wrong.

The patch set was boot tested on x86_64 and arm64 which
at the very least tests the crypto users via testmgr and
tcrypt but I do note that I do not have access to some
of the HW whose drivers are modified nor do I claim I was
able to test all of the corner cases.

The patch set is based upon linux-next release tagged
next-20170905.

Changes from v7:
- Turn -EBUSY to -EAGAIN also in crypto using net
  code which I missed before, as has been pointed
  out by Harsh Jain.

Changes from v6:
- Fix brown paper bag compile error on marvell/cesa
  code.

Changes from v5:
- Remove redundant new line as spotted by Jonathan
  Cameron.
- Reworded dm-verity change commit message to better
  clarify potential issue averted by change as
  pointed out by Mikulas Patocka.

Changes from v4:
- Rebase on top of latest algif changes from Stephan
  Mueller.
- Fix typo in ccp patch title.

Changes from v3:
- Instead of changing the return code to indicate
  backlog queueing, change the return code to indicate
  transient busy state, as suggested by Herbert Xu.

Changes from v2:
- Patch title changed from "introduce crypto wait for
  async op" to better reflect the current state.
- Rebase on top of latest linux-next.
- Add a new return code of -EIOCBQUEUED for backlog
  queueing, as suggested by Herbert Xu.
- Transform more users to the new API.
- Update the drbg change to account for new init as
  indicated by Stephan Muller.

Changes from v1:
- Address review comments from Eric Biggers.
- Separated out bug fixes of existing code and rebase
  on top of that patch set.
- Rename 'ecr' to 'wait' in fscrypto code.
- Split patch introducing the new API from the change
  moving over the algif code which it originated from
  to the new API.
- Inline crypto_wait_req().
- Some code indentation fixes.

Gilad Ben-Yossef (20):
  crypto: change transient busy return code to -EAGAIN
  crypto: ccp: use -EAGAIN for transient busy indication
  net: use -EAGAIN for transient busy indication
  crypto: remove redundant backlog checks on EBUSY
  crypto: marvell/cesa: remove redundant backlog checks on EBUSY
  crypto: introduce crypto wait for async op
  crypto: move algif to generic async completion
  crypto: move pub key to generic async completion
  crypto: move drbg to generic async completion
  crypto: move gcm to generic async completion
  crypto: move testmgr to generic async completion
  fscrypt: move to generic async completion
  dm: move dm-verity to generic async completion
  cifs: move to generic async completion
  ima: move to generic async completion
  crypto: tcrypt: move to generic async completion
  crypto: talitos: move to generic async completion
  crypto: qce: move to generic async completion
  crypto: mediatek: move to generic async completion
  crypto: adapt api sample to use async. op wait

 Documentation/crypto/api-samples.rst |  52 ++---
 crypto/af_alg.c  |  27 -
 crypto/ahash.c   |  12 +--
 crypto/algapi.c  |   6 +-
 crypto/algif_aead.c  |   8 +-
 crypto/algif_hash.c  |  50 +
 crypto/algif_skcipher.c  |   9 +-
 crypto/api.c |  13 +++
 crypto/asymmetric_keys/public_key.c  |  28 +
 crypto/cryptd.c  |   4 +-
 crypto/cts.c |   6 +-
 crypto/drbg.c|  36 ++-
 crypto/gcm.c |  32 ++
 crypto/lrw.c |   8 +-
 crypto/rsa-pkcs1pad.c|  16 +--
 crypto/tcrypt.c  |  84 +--
 crypto/testmgr.c | 204 ---
 crypto/xts.c |   8 +-
 drivers/crypto/ccp/ccp-crypto-main.c |   8 +-
 drivers/crypto/ccp/ccp-dev.c |   7 +-
 drivers/crypto/marvell/cesa.c|   3 +-
 drivers/crypto/marvell/cesa.h|   2 +-
 drivers/crypto/mediatek/mtk-aes.c|  31 +-
 drivers/crypto/qce/sha.c |  30 +-
 drivers/crypto/talitos.c |  38 +--
 drivers/md/dm-verity-target.c|  81 --
 drivers/md/dm-verity.h   |   5 -
 fs/cifs/smb2ops.c|  30 +-
 fs/crypto/crypto.c   |  28 +
 fs/crypto/fn

[PATCH v8 03/20] net: use -EAGAIN for transient busy indication

[Gilad Ben-Yossef]

Replace -EBUSY with -EAGAIN when handling transient busy
indication in the absence of backlog.

Signed-off-by: Gilad Ben-Yossef 

---

Please squash this patch with the previous one when merging upstream.
---
 net/ipv4/ah4.c  | 2 +-
 net/ipv4/esp4.c | 2 +-
 net/ipv6/ah6.c  | 2 +-
 net/ipv6/esp6.c | 2 +-
 4 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/net/ipv4/ah4.c b/net/ipv4/ah4.c
index 37db44f..049cb0a 100644
--- a/net/ipv4/ah4.c
+++ b/net/ipv4/ah4.c
@@ -240,7 +240,7 @@ static int ah_output(struct xfrm_state *x, struct sk_buff 
*skb)
if (err == -EINPROGRESS)
goto out;
 
-   if (err == -EBUSY)
+   if (err == -EAGAIN)
err = NET_XMIT_DROP;
goto out_free;
}
diff --git a/net/ipv4/esp4.c b/net/ipv4/esp4.c
index b00e4a4..ff8e088 100644
--- a/net/ipv4/esp4.c
+++ b/net/ipv4/esp4.c
@@ -432,7 +432,7 @@ int esp_output_tail(struct xfrm_state *x, struct sk_buff 
*skb, struct esp_info *
case -EINPROGRESS:
goto error;
 
-   case -EBUSY:
+   case -EAGAIN:
err = NET_XMIT_DROP;
break;
 
diff --git a/net/ipv6/ah6.c b/net/ipv6/ah6.c
index 7802b72..ba0c145 100644
--- a/net/ipv6/ah6.c
+++ b/net/ipv6/ah6.c
@@ -443,7 +443,7 @@ static int ah6_output(struct xfrm_state *x, struct sk_buff 
*skb)
if (err == -EINPROGRESS)
goto out;
 
-   if (err == -EBUSY)
+   if (err == -EAGAIN)
err = NET_XMIT_DROP;
goto out_free;
}
diff --git a/net/ipv6/esp6.c b/net/ipv6/esp6.c
index 89910e2..1a71ee5 100644
--- a/net/ipv6/esp6.c
+++ b/net/ipv6/esp6.c
@@ -396,7 +396,7 @@ int esp6_output_tail(struct xfrm_state *x, struct sk_buff 
*skb, struct esp_info
case -EINPROGRESS:
goto error;
 
-   case -EBUSY:
+   case -EAGAIN:
err = NET_XMIT_DROP;
break;
 
-- 
2.1.4

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

[PATCH v8 01/20] crypto: change transient busy return code to -EAGAIN

[Gilad Ben-Yossef]

The crypto API was using the -EBUSY return value to indicate
both a hard failure to submit a crypto operation into a
transformation provider when the latter was busy and the backlog
mechanism was not enabled as well as a notification that the
operation was queued into the backlog when the backlog mechanism
was enabled.

Having the same return code indicate two very different conditions
depending on a flag is both error prone and requires extra runtime
check like the following to discern between the cases:

if (err == -EINPROGRESS ||
(err == -EBUSY && (ahash_request_flags(req) &
   CRYPTO_TFM_REQ_MAY_BACKLOG)))

This patch changes the return code used to indicate a crypto op
failed due to the transformation provider being transiently busy
to -EAGAIN.

Signed-off-by: Gilad Ben-Yossef 
---
 crypto/algapi.c |  6 --
 crypto/algif_hash.c | 20 +---
 crypto/cryptd.c |  4 +---
 3 files changed, 22 insertions(+), 8 deletions(-)

diff --git a/crypto/algapi.c b/crypto/algapi.c
index aa699ff..916bee3 100644
--- a/crypto/algapi.c
+++ b/crypto/algapi.c
@@ -897,9 +897,11 @@ int crypto_enqueue_request(struct crypto_queue *queue,
int err = -EINPROGRESS;
 
if (unlikely(queue->qlen >= queue->max_qlen)) {
-   err = -EBUSY;
-   if (!(request->flags & CRYPTO_TFM_REQ_MAY_BACKLOG))
+   if (!(request->flags & CRYPTO_TFM_REQ_MAY_BACKLOG)) {
+   err = -EAGAIN;
goto out;
+   }
+   err = -EBUSY;
if (queue->backlog == &queue->list)
queue->backlog = &request->list;
}
diff --git a/crypto/algif_hash.c b/crypto/algif_hash.c
index 5e92bd2..3b3c154 100644
--- a/crypto/algif_hash.c
+++ b/crypto/algif_hash.c
@@ -39,6 +39,20 @@ struct algif_hash_tfm {
bool has_key;
 };
 
+/* Previous versions of crypto_* ops used to return -EBUSY
+ * rather than -EAGAIN to indicate being tied up. The in
+ * kernel API changed but we don't want to break the user
+ * space API. As only the hash user interface exposed this
+ * error ever to the user, do the translation here.
+ */
+static inline int crypto_user_err(int err)
+{
+   if (err == -EAGAIN)
+   return -EBUSY;
+
+   return err;
+}
+
 static int hash_alloc_result(struct sock *sk, struct hash_ctx *ctx)
 {
unsigned ds;
@@ -136,7 +150,7 @@ static int hash_sendmsg(struct socket *sock, struct msghdr 
*msg,
 unlock:
release_sock(sk);
 
-   return err ?: copied;
+   return err ? crypto_user_err(err) : copied;
 }
 
 static ssize_t hash_sendpage(struct socket *sock, struct page *page,
@@ -188,7 +202,7 @@ static ssize_t hash_sendpage(struct socket *sock, struct 
page *page,
 unlock:
release_sock(sk);
 
-   return err ?: size;
+   return err ? crypto_user_err(err) : size;
 }
 
 static int hash_recvmsg(struct socket *sock, struct msghdr *msg, size_t len,
@@ -236,7 +250,7 @@ static int hash_recvmsg(struct socket *sock, struct msghdr 
*msg, size_t len,
hash_free_result(sk, ctx);
release_sock(sk);
 
-   return err ?: len;
+   return err ? crypto_user_err(err) : len;
 }
 
 static int hash_accept(struct socket *sock, struct socket *newsock, int flags,
diff --git a/crypto/cryptd.c b/crypto/cryptd.c
index 0508c48..d1dbdce 100644
--- a/crypto/cryptd.c
+++ b/crypto/cryptd.c
@@ -137,16 +137,14 @@ static int cryptd_enqueue_request(struct cryptd_queue 
*queue,
int cpu, err;
struct cryptd_cpu_queue *cpu_queue;
atomic_t *refcnt;
-   bool may_backlog;
 
cpu = get_cpu();
cpu_queue = this_cpu_ptr(queue->cpu_queue);
err = crypto_enqueue_request(&cpu_queue->queue, request);
 
refcnt = crypto_tfm_ctx(request->tfm);
-   may_backlog = request->flags & CRYPTO_TFM_REQ_MAY_BACKLOG;
 
-   if (err == -EBUSY && !may_backlog)
+   if (err == -EAGAIN)
goto out_put_cpu;
 
queue_work_on(cpu, kcrypto_wq, &cpu_queue->work);
-- 
2.1.4

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

[PATCH v8 05/20] crypto: marvell/cesa: remove redundant backlog checks on EBUSY

[Gilad Ben-Yossef]

Now that -EBUSY return code only indicates backlog queueing
we can safely remove the now redundant check for the
CRYPTO_TFM_REQ_MAY_BACKLOG flag when -EBUSY is returned.

Signed-off-by: Gilad Ben-Yossef 
Acked-by: Boris Brezillon 
---
 drivers/crypto/marvell/cesa.c | 3 +--
 drivers/crypto/marvell/cesa.h | 2 +-
 2 files changed, 2 insertions(+), 3 deletions(-)

diff --git a/drivers/crypto/marvell/cesa.c b/drivers/crypto/marvell/cesa.c
index 6e7a5c7..0c3909d 100644
--- a/drivers/crypto/marvell/cesa.c
+++ b/drivers/crypto/marvell/cesa.c
@@ -183,8 +183,7 @@ int mv_cesa_queue_req(struct crypto_async_request *req,
spin_lock_bh(&engine->lock);
ret = crypto_enqueue_request(&engine->queue, req);
if ((mv_cesa_req_get_type(creq) == CESA_DMA_REQ) &&
-   (ret == -EINPROGRESS ||
-   (ret == -EBUSY && req->flags & CRYPTO_TFM_REQ_MAY_BACKLOG)))
+   (ret == -EINPROGRESS || ret == -EBUSY))
mv_cesa_tdma_chain(engine, creq);
spin_unlock_bh(&engine->lock);
 
diff --git a/drivers/crypto/marvell/cesa.h b/drivers/crypto/marvell/cesa.h
index b7872f6..63c8457 100644
--- a/drivers/crypto/marvell/cesa.h
+++ b/drivers/crypto/marvell/cesa.h
@@ -763,7 +763,7 @@ static inline int mv_cesa_req_needs_cleanup(struct 
crypto_async_request *req,
 * the backlog and will be processed later. There's no need to
 * clean it up.
 */
-   if (ret == -EBUSY && req->flags & CRYPTO_TFM_REQ_MAY_BACKLOG)
+   if (ret == -EBUSY)
return false;
 
/* Request wasn't queued, we need to clean it up */
-- 
2.1.4

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

[PATCH v8 06/20] crypto: introduce crypto wait for async op

[Gilad Ben-Yossef]

Invoking a possibly async. crypto op and waiting for completion
while correctly handling backlog processing is a common task
in the crypto API implementation and outside users of it.

This patch adds a generic implementation for doing so in
preparation for using it across the board instead of hand
rolled versions.

Signed-off-by: Gilad Ben-Yossef 
CC: Eric Biggers 
CC: Jonathan Cameron 
---
 crypto/api.c   | 13 +
 include/linux/crypto.h | 40 
 2 files changed, 53 insertions(+)

diff --git a/crypto/api.c b/crypto/api.c
index 941cd4c..2a2479d 100644
--- a/crypto/api.c
+++ b/crypto/api.c
@@ -24,6 +24,7 @@
 #include 
 #include 
 #include 
+#include 
 #include "internal.h"
 
 LIST_HEAD(crypto_alg_list);
@@ -595,5 +596,17 @@ int crypto_has_alg(const char *name, u32 type, u32 mask)
 }
 EXPORT_SYMBOL_GPL(crypto_has_alg);
 
+void crypto_req_done(struct crypto_async_request *req, int err)
+{
+   struct crypto_wait *wait = req->data;
+
+   if (err == -EINPROGRESS)
+   return;
+
+   wait->err = err;
+   complete(&wait->completion);
+}
+EXPORT_SYMBOL_GPL(crypto_req_done);
+
 MODULE_DESCRIPTION("Cryptographic core API");
 MODULE_LICENSE("GPL");
diff --git a/include/linux/crypto.h b/include/linux/crypto.h
index 84da997..78508ca 100644
--- a/include/linux/crypto.h
+++ b/include/linux/crypto.h
@@ -24,6 +24,7 @@
 #include 
 #include 
 #include 
+#include 
 
 /*
  * Autoloaded crypto modules should only use a prefixed name to avoid allowing
@@ -468,6 +469,45 @@ struct crypto_alg {
 } CRYPTO_MINALIGN_ATTR;
 
 /*
+ * A helper struct for waiting for completion of async crypto ops
+ */
+struct crypto_wait {
+   struct completion completion;
+   int err;
+};
+
+/*
+ * Macro for declaring a crypto op async wait object on stack
+ */
+#define DECLARE_CRYPTO_WAIT(_wait) \
+   struct crypto_wait _wait = { \
+   COMPLETION_INITIALIZER_ONSTACK((_wait).completion), 0 }
+
+/*
+ * Async ops completion helper functioons
+ */
+void crypto_req_done(struct crypto_async_request *req, int err);
+
+static inline int crypto_wait_req(int err, struct crypto_wait *wait)
+{
+   switch (err) {
+   case -EINPROGRESS:
+   case -EBUSY:
+   wait_for_completion(&wait->completion);
+   reinit_completion(&wait->completion);
+   err = wait->err;
+   break;
+   };
+
+   return err;
+}
+
+static inline void crypto_init_wait(struct crypto_wait *wait)
+{
+   init_completion(&wait->completion);
+}
+
+/*
  * Algorithm registration interface.
  */
 int crypto_register_alg(struct crypto_alg *alg);
-- 
2.1.4

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

[PATCH v8 08/20] crypto: move pub key to generic async completion

[Gilad Ben-Yossef]

public_key_verify_signature() is starting an async crypto op and
waiting for it to complete. Move it over to generic code doing
the same.

Signed-off-by: Gilad Ben-Yossef 
---
 crypto/asymmetric_keys/public_key.c | 28 
 1 file changed, 4 insertions(+), 24 deletions(-)

diff --git a/crypto/asymmetric_keys/public_key.c 
b/crypto/asymmetric_keys/public_key.c
index 3cd6e12..d916235 100644
--- a/crypto/asymmetric_keys/public_key.c
+++ b/crypto/asymmetric_keys/public_key.c
@@ -57,29 +57,13 @@ static void public_key_destroy(void *payload0, void 
*payload3)
public_key_signature_free(payload3);
 }
 
-struct public_key_completion {
-   struct completion completion;
-   int err;
-};
-
-static void public_key_verify_done(struct crypto_async_request *req, int err)
-{
-   struct public_key_completion *compl = req->data;
-
-   if (err == -EINPROGRESS)
-   return;
-
-   compl->err = err;
-   complete(&compl->completion);
-}
-
 /*
  * Verify a signature using a public key.
  */
 int public_key_verify_signature(const struct public_key *pkey,
const struct public_key_signature *sig)
 {
-   struct public_key_completion compl;
+   struct crypto_wait cwait;
struct crypto_akcipher *tfm;
struct akcipher_request *req;
struct scatterlist sig_sg, digest_sg;
@@ -131,20 +115,16 @@ int public_key_verify_signature(const struct public_key 
*pkey,
sg_init_one(&digest_sg, output, outlen);
akcipher_request_set_crypt(req, &sig_sg, &digest_sg, sig->s_size,
   outlen);
-   init_completion(&compl.completion);
+   crypto_init_wait(&cwait);
akcipher_request_set_callback(req, CRYPTO_TFM_REQ_MAY_BACKLOG |
  CRYPTO_TFM_REQ_MAY_SLEEP,
- public_key_verify_done, &compl);
+ crypto_req_done, &cwait);
 
/* Perform the verification calculation.  This doesn't actually do the
 * verification, but rather calculates the hash expected by the
 * signature and returns that to us.
 */
-   ret = crypto_akcipher_verify(req);
-   if ((ret == -EINPROGRESS) || (ret == -EBUSY)) {
-   wait_for_completion(&compl.completion);
-   ret = compl.err;
-   }
+   ret = crypto_wait_req(crypto_akcipher_verify(req), &cwait);
if (ret < 0)
goto out_free_output;
 
-- 
2.1.4

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

[PATCH v8 07/20] crypto: move algif to generic async completion

[Gilad Ben-Yossef]

algif starts several async crypto ops and waits for their completion.
Move it over to generic code doing the same.

Signed-off-by: Gilad Ben-Yossef 
---
 crypto/af_alg.c | 27 ---
 crypto/algif_aead.c |  8 
 crypto/algif_hash.c | 30 ++
 crypto/algif_skcipher.c |  9 -
 include/crypto/if_alg.h | 15 +--
 5 files changed, 23 insertions(+), 66 deletions(-)

diff --git a/crypto/af_alg.c b/crypto/af_alg.c
index ffa9f4c..cf312ed 100644
--- a/crypto/af_alg.c
+++ b/crypto/af_alg.c
@@ -481,33 +481,6 @@ int af_alg_cmsg_send(struct msghdr *msg, struct 
af_alg_control *con)
 }
 EXPORT_SYMBOL_GPL(af_alg_cmsg_send);
 
-int af_alg_wait_for_completion(int err, struct af_alg_completion *completion)
-{
-   switch (err) {
-   case -EINPROGRESS:
-   case -EBUSY:
-   wait_for_completion(&completion->completion);
-   reinit_completion(&completion->completion);
-   err = completion->err;
-   break;
-   };
-
-   return err;
-}
-EXPORT_SYMBOL_GPL(af_alg_wait_for_completion);
-
-void af_alg_complete(struct crypto_async_request *req, int err)
-{
-   struct af_alg_completion *completion = req->data;
-
-   if (err == -EINPROGRESS)
-   return;
-
-   completion->err = err;
-   complete(&completion->completion);
-}
-EXPORT_SYMBOL_GPL(af_alg_complete);
-
 /**
  * af_alg_alloc_tsgl - allocate the TX SGL
  *
diff --git a/crypto/algif_aead.c b/crypto/algif_aead.c
index 516b38c..aacae08 100644
--- a/crypto/algif_aead.c
+++ b/crypto/algif_aead.c
@@ -278,11 +278,11 @@ static int _aead_recvmsg(struct socket *sock, struct 
msghdr *msg,
/* Synchronous operation */
aead_request_set_callback(&areq->cra_u.aead_req,
  CRYPTO_TFM_REQ_MAY_BACKLOG,
- af_alg_complete, &ctx->completion);
-   err = af_alg_wait_for_completion(ctx->enc ?
+ crypto_req_done, &ctx->wait);
+   err = crypto_wait_req(ctx->enc ?
crypto_aead_encrypt(&areq->cra_u.aead_req) :
crypto_aead_decrypt(&areq->cra_u.aead_req),
-&ctx->completion);
+   &ctx->wait);
}
 
/* AIO operation in progress */
@@ -554,7 +554,7 @@ static int aead_accept_parent_nokey(void *private, struct 
sock *sk)
ctx->merge = 0;
ctx->enc = 0;
ctx->aead_assoclen = 0;
-   af_alg_init_completion(&ctx->completion);
+   crypto_init_wait(&ctx->wait);
 
ask->private = ctx;
 
diff --git a/crypto/algif_hash.c b/crypto/algif_hash.c
index 3b3c154..d2ab8de 100644
--- a/crypto/algif_hash.c
+++ b/crypto/algif_hash.c
@@ -26,7 +26,7 @@ struct hash_ctx {
 
u8 *result;
 
-   struct af_alg_completion completion;
+   struct crypto_wait wait;
 
unsigned int len;
bool more;
@@ -102,8 +102,7 @@ static int hash_sendmsg(struct socket *sock, struct msghdr 
*msg,
if ((msg->msg_flags & MSG_MORE))
hash_free_result(sk, ctx);
 
-   err = af_alg_wait_for_completion(crypto_ahash_init(&ctx->req),
-   &ctx->completion);
+   err = crypto_wait_req(crypto_ahash_init(&ctx->req), &ctx->wait);
if (err)
goto unlock;
}
@@ -124,8 +123,8 @@ static int hash_sendmsg(struct socket *sock, struct msghdr 
*msg,
 
ahash_request_set_crypt(&ctx->req, ctx->sgl.sg, NULL, len);
 
-   err = af_alg_wait_for_completion(crypto_ahash_update(&ctx->req),
-&ctx->completion);
+   err = crypto_wait_req(crypto_ahash_update(&ctx->req),
+ &ctx->wait);
af_alg_free_sg(&ctx->sgl);
if (err)
goto unlock;
@@ -143,8 +142,8 @@ static int hash_sendmsg(struct socket *sock, struct msghdr 
*msg,
goto unlock;
 
ahash_request_set_crypt(&ctx->req, NULL, ctx->result, 0);
-   err = af_alg_wait_for_completion(crypto_ahash_final(&ctx->req),
-&ctx->completion);
+   err = crypto_wait_req(crypto_ahash_final(&ctx->req),
+ &ctx->wait);
}
 
 unlock:
@@ -185,7 +184,7 @@ static ssize_t hash_sendpage(struct socket *sock, struct 
page *page,
} else {
if (!ctx->more) {
err = crypto_ahash_init(&ctx->req);
-   err = af_alg_wait_for_completion(err, &ctx->completion);
+   err = crypto_wait_req(err, &ctx->wait);
if (err)
 

[PATCH v8 12/20] fscrypt: move to generic async completion

[Gilad Ben-Yossef]

fscrypt starts several async. crypto ops and waiting for them to
complete. Move it over to generic code doing the same.

Signed-off-by: Gilad Ben-Yossef 
---
 fs/crypto/crypto.c  | 28 
 fs/crypto/fname.c   | 36 ++--
 fs/crypto/fscrypt_private.h | 10 --
 fs/crypto/keyinfo.c | 21 +++--
 4 files changed, 13 insertions(+), 82 deletions(-)

diff --git a/fs/crypto/crypto.c b/fs/crypto/crypto.c
index c7835df..80a3cad 100644
--- a/fs/crypto/crypto.c
+++ b/fs/crypto/crypto.c
@@ -126,21 +126,6 @@ struct fscrypt_ctx *fscrypt_get_ctx(const struct inode 
*inode, gfp_t gfp_flags)
 }
 EXPORT_SYMBOL(fscrypt_get_ctx);
 
-/**
- * page_crypt_complete() - completion callback for page crypto
- * @req: The asynchronous cipher request context
- * @res: The result of the cipher operation
- */
-static void page_crypt_complete(struct crypto_async_request *req, int res)
-{
-   struct fscrypt_completion_result *ecr = req->data;
-
-   if (res == -EINPROGRESS)
-   return;
-   ecr->res = res;
-   complete(&ecr->completion);
-}
-
 int fscrypt_do_page_crypto(const struct inode *inode, fscrypt_direction_t rw,
   u64 lblk_num, struct page *src_page,
   struct page *dest_page, unsigned int len,
@@ -151,7 +136,7 @@ int fscrypt_do_page_crypto(const struct inode *inode, 
fscrypt_direction_t rw,
u8 padding[FS_IV_SIZE - sizeof(__le64)];
} iv;
struct skcipher_request *req = NULL;
-   DECLARE_FS_COMPLETION_RESULT(ecr);
+   DECLARE_CRYPTO_WAIT(wait);
struct scatterlist dst, src;
struct fscrypt_info *ci = inode->i_crypt_info;
struct crypto_skcipher *tfm = ci->ci_ctfm;
@@ -179,7 +164,7 @@ int fscrypt_do_page_crypto(const struct inode *inode, 
fscrypt_direction_t rw,
 
skcipher_request_set_callback(
req, CRYPTO_TFM_REQ_MAY_BACKLOG | CRYPTO_TFM_REQ_MAY_SLEEP,
-   page_crypt_complete, &ecr);
+   crypto_req_done, &wait);
 
sg_init_table(&dst, 1);
sg_set_page(&dst, dest_page, len, offs);
@@ -187,14 +172,9 @@ int fscrypt_do_page_crypto(const struct inode *inode, 
fscrypt_direction_t rw,
sg_set_page(&src, src_page, len, offs);
skcipher_request_set_crypt(req, &src, &dst, len, &iv);
if (rw == FS_DECRYPT)
-   res = crypto_skcipher_decrypt(req);
+   res = crypto_wait_req(crypto_skcipher_decrypt(req), &wait);
else
-   res = crypto_skcipher_encrypt(req);
-   if (res == -EINPROGRESS || res == -EBUSY) {
-   BUG_ON(req->base.data != &ecr);
-   wait_for_completion(&ecr.completion);
-   res = ecr.res;
-   }
+   res = crypto_wait_req(crypto_skcipher_encrypt(req), &wait);
skcipher_request_free(req);
if (res) {
printk_ratelimited(KERN_ERR
diff --git a/fs/crypto/fname.c b/fs/crypto/fname.c
index ad9f814..a80a0d3 100644
--- a/fs/crypto/fname.c
+++ b/fs/crypto/fname.c
@@ -15,21 +15,6 @@
 #include "fscrypt_private.h"
 
 /**
- * fname_crypt_complete() - completion callback for filename crypto
- * @req: The asynchronous cipher request context
- * @res: The result of the cipher operation
- */
-static void fname_crypt_complete(struct crypto_async_request *req, int res)
-{
-   struct fscrypt_completion_result *ecr = req->data;
-
-   if (res == -EINPROGRESS)
-   return;
-   ecr->res = res;
-   complete(&ecr->completion);
-}
-
-/**
  * fname_encrypt() - encrypt a filename
  *
  * The caller must have allocated sufficient memory for the @oname string.
@@ -40,7 +25,7 @@ static int fname_encrypt(struct inode *inode,
const struct qstr *iname, struct fscrypt_str *oname)
 {
struct skcipher_request *req = NULL;
-   DECLARE_FS_COMPLETION_RESULT(ecr);
+   DECLARE_CRYPTO_WAIT(wait);
struct fscrypt_info *ci = inode->i_crypt_info;
struct crypto_skcipher *tfm = ci->ci_ctfm;
int res = 0;
@@ -76,17 +61,12 @@ static int fname_encrypt(struct inode *inode,
}
skcipher_request_set_callback(req,
CRYPTO_TFM_REQ_MAY_BACKLOG | CRYPTO_TFM_REQ_MAY_SLEEP,
-   fname_crypt_complete, &ecr);
+   crypto_req_done, &wait);
sg_init_one(&sg, oname->name, cryptlen);
skcipher_request_set_crypt(req, &sg, &sg, cryptlen, iv);
 
/* Do the encryption */
-   res = crypto_skcipher_encrypt(req);
-   if (res == -EINPROGRESS || res == -EBUSY) {
-   /* Request is being completed asynchronously; wait for it */
-   wait_for_completion(&ecr.completion);
-   res = ecr.res;
-   }
+   res = crypto_wait_req(crypto_skcipher_encrypt(req), &wait);
skcipher_request_free(req);
if (res < 0) {
printk_ratelimite

[PATCH v8 13/20] dm: move dm-verity to generic async completion

[Gilad Ben-Yossef]

dm-verity is starting async. crypto ops and waiting for them to complete.
Move it over to generic code doing the same.

This also avoids a future potential data coruption bug created
by the use of wait_for_completion_interruptible() without dealing
correctly with an interrupt aborting the wait prior to the
async op finishing, should this code ever move to a context
where signals are not masked.

Signed-off-by: Gilad Ben-Yossef 
CC: Mikulas Patocka 
---
 drivers/md/dm-verity-target.c | 81 +++
 drivers/md/dm-verity.h|  5 ---
 2 files changed, 20 insertions(+), 66 deletions(-)

diff --git a/drivers/md/dm-verity-target.c b/drivers/md/dm-verity-target.c
index bda3cac..811ad28 100644
--- a/drivers/md/dm-verity-target.c
+++ b/drivers/md/dm-verity-target.c
@@ -92,74 +92,33 @@ static sector_t verity_position_at_level(struct dm_verity 
*v, sector_t block,
return block >> (level * v->hash_per_block_bits);
 }
 
-/*
- * Callback function for asynchrnous crypto API completion notification
- */
-static void verity_op_done(struct crypto_async_request *base, int err)
-{
-   struct verity_result *res = (struct verity_result *)base->data;
-
-   if (err == -EINPROGRESS)
-   return;
-
-   res->err = err;
-   complete(&res->completion);
-}
-
-/*
- * Wait for async crypto API callback
- */
-static inline int verity_complete_op(struct verity_result *res, int ret)
-{
-   switch (ret) {
-   case 0:
-   break;
-
-   case -EINPROGRESS:
-   case -EBUSY:
-   ret = wait_for_completion_interruptible(&res->completion);
-   if (!ret)
-   ret = res->err;
-   reinit_completion(&res->completion);
-   break;
-
-   default:
-   DMERR("verity_wait_hash: crypto op submission failed: %d", ret);
-   }
-
-   if (unlikely(ret < 0))
-   DMERR("verity_wait_hash: crypto op failed: %d", ret);
-
-   return ret;
-}
-
 static int verity_hash_update(struct dm_verity *v, struct ahash_request *req,
const u8 *data, size_t len,
-   struct verity_result *res)
+   struct crypto_wait *wait)
 {
struct scatterlist sg;
 
sg_init_one(&sg, data, len);
ahash_request_set_crypt(req, &sg, NULL, len);
 
-   return verity_complete_op(res, crypto_ahash_update(req));
+   return crypto_wait_req(crypto_ahash_update(req), wait);
 }
 
 /*
  * Wrapper for crypto_ahash_init, which handles verity salting.
  */
 static int verity_hash_init(struct dm_verity *v, struct ahash_request *req,
-   struct verity_result *res)
+   struct crypto_wait *wait)
 {
int r;
 
ahash_request_set_tfm(req, v->tfm);
ahash_request_set_callback(req, CRYPTO_TFM_REQ_MAY_SLEEP |
CRYPTO_TFM_REQ_MAY_BACKLOG,
-   verity_op_done, (void *)res);
-   init_completion(&res->completion);
+   crypto_req_done, (void *)wait);
+   crypto_init_wait(wait);
 
-   r = verity_complete_op(res, crypto_ahash_init(req));
+   r = crypto_wait_req(crypto_ahash_init(req), wait);
 
if (unlikely(r < 0)) {
DMERR("crypto_ahash_init failed: %d", r);
@@ -167,18 +126,18 @@ static int verity_hash_init(struct dm_verity *v, struct 
ahash_request *req,
}
 
if (likely(v->salt_size && (v->version >= 1)))
-   r = verity_hash_update(v, req, v->salt, v->salt_size, res);
+   r = verity_hash_update(v, req, v->salt, v->salt_size, wait);
 
return r;
 }
 
 static int verity_hash_final(struct dm_verity *v, struct ahash_request *req,
-u8 *digest, struct verity_result *res)
+u8 *digest, struct crypto_wait *wait)
 {
int r;
 
if (unlikely(v->salt_size && (!v->version))) {
-   r = verity_hash_update(v, req, v->salt, v->salt_size, res);
+   r = verity_hash_update(v, req, v->salt, v->salt_size, wait);
 
if (r < 0) {
DMERR("verity_hash_final failed updating salt: %d", r);
@@ -187,7 +146,7 @@ static int verity_hash_final(struct dm_verity *v, struct 
ahash_request *req,
}
 
ahash_request_set_crypt(req, NULL, digest, 0);
-   r = verity_complete_op(res, crypto_ahash_final(req));
+   r = crypto_wait_req(crypto_ahash_final(req), wait);
 out:
return r;
 }
@@ -196,17 +155,17 @@ int verity_hash(struct dm_verity *v, struct ahash_request 
*req,
const u8 *data, size_t len, u8 *digest)
 {
int r;
-   struct verity_result res;
+   struct crypto_wait wait;
 
-   r = verity_hash_init(v, req, &res);
+   r = verity_hash_init(v, req, &wait);
if (unlikely(r < 0))
  

[PATCH v8 16/20] crypto: tcrypt: move to generic async completion

[Gilad Ben-Yossef]

tcrypt starts several async crypto ops and  waits for their completions.
Move it over to generic code doing the same.

Signed-off-by: Gilad Ben-Yossef 
---
 crypto/tcrypt.c | 84 +
 1 file changed, 25 insertions(+), 59 deletions(-)

diff --git a/crypto/tcrypt.c b/crypto/tcrypt.c
index 0022a18..802aa81 100644
--- a/crypto/tcrypt.c
+++ b/crypto/tcrypt.c
@@ -79,34 +79,11 @@ static char *check[] = {
NULL
 };
 
-struct tcrypt_result {
-   struct completion completion;
-   int err;
-};
-
-static void tcrypt_complete(struct crypto_async_request *req, int err)
-{
-   struct tcrypt_result *res = req->data;
-
-   if (err == -EINPROGRESS)
-   return;
-
-   res->err = err;
-   complete(&res->completion);
-}
-
 static inline int do_one_aead_op(struct aead_request *req, int ret)
 {
-   if (ret == -EINPROGRESS || ret == -EBUSY) {
-   struct tcrypt_result *tr = req->base.data;
+   struct crypto_wait *wait = req->base.data;
 
-   ret = wait_for_completion_interruptible(&tr->completion);
-   if (!ret)
-   ret = tr->err;
-   reinit_completion(&tr->completion);
-   }
-
-   return ret;
+   return crypto_wait_req(ret, wait);
 }
 
 static int test_aead_jiffies(struct aead_request *req, int enc,
@@ -248,7 +225,7 @@ static void test_aead_speed(const char *algo, int enc, 
unsigned int secs,
char *axbuf[XBUFSIZE];
unsigned int *b_size;
unsigned int iv_len;
-   struct tcrypt_result result;
+   struct crypto_wait wait;
 
iv = kzalloc(MAX_IVLEN, GFP_KERNEL);
if (!iv)
@@ -284,7 +261,7 @@ static void test_aead_speed(const char *algo, int enc, 
unsigned int secs,
goto out_notfm;
}
 
-   init_completion(&result.completion);
+   crypto_init_wait(&wait);
printk(KERN_INFO "\ntesting speed of %s (%s) %s\n", algo,
get_driver_name(crypto_aead, tfm), e);
 
@@ -296,7 +273,7 @@ static void test_aead_speed(const char *algo, int enc, 
unsigned int secs,
}
 
aead_request_set_callback(req, CRYPTO_TFM_REQ_MAY_BACKLOG,
- tcrypt_complete, &result);
+ crypto_req_done, &wait);
 
i = 0;
do {
@@ -397,21 +374,16 @@ static void test_hash_sg_init(struct scatterlist *sg)
 
 static inline int do_one_ahash_op(struct ahash_request *req, int ret)
 {
-   if (ret == -EINPROGRESS || ret == -EBUSY) {
-   struct tcrypt_result *tr = req->base.data;
+   struct crypto_wait *wait = req->base.data;
 
-   wait_for_completion(&tr->completion);
-   reinit_completion(&tr->completion);
-   ret = tr->err;
-   }
-   return ret;
+   return crypto_wait_req(ret, wait);
 }
 
 struct test_mb_ahash_data {
struct scatterlist sg[TVMEMSIZE];
char result[64];
struct ahash_request *req;
-   struct tcrypt_result tresult;
+   struct crypto_wait wait;
char *xbuf[XBUFSIZE];
 };
 
@@ -440,7 +412,7 @@ static void test_mb_ahash_speed(const char *algo, unsigned 
int sec,
if (testmgr_alloc_buf(data[i].xbuf))
goto out;
 
-   init_completion(&data[i].tresult.completion);
+   crypto_init_wait(&data[i].wait);
 
data[i].req = ahash_request_alloc(tfm, GFP_KERNEL);
if (!data[i].req) {
@@ -449,8 +421,8 @@ static void test_mb_ahash_speed(const char *algo, unsigned 
int sec,
goto out;
}
 
-   ahash_request_set_callback(data[i].req, 0,
-  tcrypt_complete, &data[i].tresult);
+   ahash_request_set_callback(data[i].req, 0, crypto_req_done,
+  &data[i].wait);
test_hash_sg_init(data[i].sg);
}
 
@@ -492,16 +464,16 @@ static void test_mb_ahash_speed(const char *algo, 
unsigned int sec,
if (ret)
break;
 
-   complete(&data[k].tresult.completion);
-   data[k].tresult.err = 0;
+   crypto_req_done(&data[k].req->base, 0);
}
 
for (j = 0; j < k; j++) {
-   struct tcrypt_result *tr = &data[j].tresult;
+   struct crypto_wait *wait = &data[j].wait;
+   int wait_ret;
 
-   wait_for_completion(&tr->completion);
-   if (tr->err)
-   ret = tr->err;
+   wait_ret = crypto_wait_req(-EINPROGRESS, wait);
+   if (wait_ret)
+   ret = wait_ret;
}
 
end = get_cycles();
@@ -679,7 +651,7 @@ static void test_ahash_speed_comm

[PATCH v8 17/20] crypto: talitos: move to generic async completion

[Gilad Ben-Yossef]

The talitos driver starts several async crypto ops and  waits for their
completions. Move it over to generic code doing the same.

Signed-off-by: Gilad Ben-Yossef 
---
 drivers/crypto/talitos.c | 38 +-
 1 file changed, 5 insertions(+), 33 deletions(-)

diff --git a/drivers/crypto/talitos.c b/drivers/crypto/talitos.c
index 79791c6..194a307 100644
--- a/drivers/crypto/talitos.c
+++ b/drivers/crypto/talitos.c
@@ -2037,22 +2037,6 @@ static int ahash_import(struct ahash_request *areq, 
const void *in)
return 0;
 }
 
-struct keyhash_result {
-   struct completion completion;
-   int err;
-};
-
-static void keyhash_complete(struct crypto_async_request *req, int err)
-{
-   struct keyhash_result *res = req->data;
-
-   if (err == -EINPROGRESS)
-   return;
-
-   res->err = err;
-   complete(&res->completion);
-}
-
 static int keyhash(struct crypto_ahash *tfm, const u8 *key, unsigned int 
keylen,
   u8 *hash)
 {
@@ -2060,10 +2044,10 @@ static int keyhash(struct crypto_ahash *tfm, const u8 
*key, unsigned int keylen,
 
struct scatterlist sg[1];
struct ahash_request *req;
-   struct keyhash_result hresult;
+   struct crypto_wait wait;
int ret;
 
-   init_completion(&hresult.completion);
+   crypto_init_wait(&wait);
 
req = ahash_request_alloc(tfm, GFP_KERNEL);
if (!req)
@@ -2072,25 +2056,13 @@ static int keyhash(struct crypto_ahash *tfm, const u8 
*key, unsigned int keylen,
/* Keep tfm keylen == 0 during hash of the long key */
ctx->keylen = 0;
ahash_request_set_callback(req, CRYPTO_TFM_REQ_MAY_BACKLOG,
-  keyhash_complete, &hresult);
+  crypto_req_done, &wait);
 
sg_init_one(&sg[0], key, keylen);
 
ahash_request_set_crypt(req, sg, hash, keylen);
-   ret = crypto_ahash_digest(req);
-   switch (ret) {
-   case 0:
-   break;
-   case -EINPROGRESS:
-   case -EBUSY:
-   ret = wait_for_completion_interruptible(
-   &hresult.completion);
-   if (!ret)
-   ret = hresult.err;
-   break;
-   default:
-   break;
-   }
+   ret = crypto_wait_req(crypto_ahash_digest(req), &wait);
+
ahash_request_free(req);
 
return ret;
-- 
2.1.4

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

[PATCH v8 19/20] crypto: mediatek: move to generic async completion

[Gilad Ben-Yossef]

The mediatek driver starts several async crypto ops and waits for their
completions. Move it over to generic code doing the same.

Signed-off-by: Gilad Ben-Yossef 
Acked-by: Ryder Lee 
---
 drivers/crypto/mediatek/mtk-aes.c | 31 +--
 1 file changed, 5 insertions(+), 26 deletions(-)

diff --git a/drivers/crypto/mediatek/mtk-aes.c 
b/drivers/crypto/mediatek/mtk-aes.c
index 9e845e8..e2c7c95 100644
--- a/drivers/crypto/mediatek/mtk-aes.c
+++ b/drivers/crypto/mediatek/mtk-aes.c
@@ -137,11 +137,6 @@ struct mtk_aes_gcm_ctx {
struct crypto_skcipher *ctr;
 };
 
-struct mtk_aes_gcm_setkey_result {
-   int err;
-   struct completion completion;
-};
-
 struct mtk_aes_drv {
struct list_head dev_list;
/* Device list lock */
@@ -936,17 +931,6 @@ static int mtk_aes_gcm_crypt(struct aead_request *req, u64 
mode)
&req->base);
 }
 
-static void mtk_gcm_setkey_done(struct crypto_async_request *req, int err)
-{
-   struct mtk_aes_gcm_setkey_result *result = req->data;
-
-   if (err == -EINPROGRESS)
-   return;
-
-   result->err = err;
-   complete(&result->completion);
-}
-
 /*
  * Because of the hardware limitation, we need to pre-calculate key(H)
  * for the GHASH operation. The result of the encryption operation
@@ -962,7 +946,7 @@ static int mtk_aes_gcm_setkey(struct crypto_aead *aead, 
const u8 *key,
u32 hash[4];
u8 iv[8];
 
-   struct mtk_aes_gcm_setkey_result result;
+   struct crypto_wait wait;
 
struct scatterlist sg[1];
struct skcipher_request req;
@@ -1002,22 +986,17 @@ static int mtk_aes_gcm_setkey(struct crypto_aead *aead, 
const u8 *key,
if (!data)
return -ENOMEM;
 
-   init_completion(&data->result.completion);
+   crypto_init_wait(&data->wait);
sg_init_one(data->sg, &data->hash, AES_BLOCK_SIZE);
skcipher_request_set_tfm(&data->req, ctr);
skcipher_request_set_callback(&data->req, CRYPTO_TFM_REQ_MAY_SLEEP |
  CRYPTO_TFM_REQ_MAY_BACKLOG,
- mtk_gcm_setkey_done, &data->result);
+ crypto_req_done, &data->wait);
skcipher_request_set_crypt(&data->req, data->sg, data->sg,
   AES_BLOCK_SIZE, data->iv);
 
-   err = crypto_skcipher_encrypt(&data->req);
-   if (err == -EINPROGRESS || err == -EBUSY) {
-   err = wait_for_completion_interruptible(
-   &data->result.completion);
-   if (!err)
-   err = data->result.err;
-   }
+   err = crypto_wait_req(crypto_skcipher_encrypt(&data->req),
+ &data->wait);
if (err)
goto out;
 
-- 
2.1.4

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

[PATCH v8 18/20] crypto: qce: move to generic async completion

[Gilad Ben-Yossef]

The qce driver starts several async crypto ops and  waits for their
completions. Move it over to generic code doing the same.

Signed-off-by: Gilad Ben-Yossef 
---
 drivers/crypto/qce/sha.c | 30 --
 1 file changed, 4 insertions(+), 26 deletions(-)

diff --git a/drivers/crypto/qce/sha.c b/drivers/crypto/qce/sha.c
index 47e114a..53227d7 100644
--- a/drivers/crypto/qce/sha.c
+++ b/drivers/crypto/qce/sha.c
@@ -349,28 +349,12 @@ static int qce_ahash_digest(struct ahash_request *req)
return qce->async_req_enqueue(tmpl->qce, &req->base);
 }
 
-struct qce_ahash_result {
-   struct completion completion;
-   int error;
-};
-
-static void qce_digest_complete(struct crypto_async_request *req, int error)
-{
-   struct qce_ahash_result *result = req->data;
-
-   if (error == -EINPROGRESS)
-   return;
-
-   result->error = error;
-   complete(&result->completion);
-}
-
 static int qce_ahash_hmac_setkey(struct crypto_ahash *tfm, const u8 *key,
 unsigned int keylen)
 {
unsigned int digestsize = crypto_ahash_digestsize(tfm);
struct qce_sha_ctx *ctx = crypto_tfm_ctx(&tfm->base);
-   struct qce_ahash_result result;
+   struct crypto_wait wait;
struct ahash_request *req;
struct scatterlist sg;
unsigned int blocksize;
@@ -405,9 +389,9 @@ static int qce_ahash_hmac_setkey(struct crypto_ahash *tfm, 
const u8 *key,
goto err_free_ahash;
}
 
-   init_completion(&result.completion);
+   crypto_init_wait(&wait);
ahash_request_set_callback(req, CRYPTO_TFM_REQ_MAY_BACKLOG,
-  qce_digest_complete, &result);
+  crypto_req_done, &wait);
crypto_ahash_clear_flags(ahash_tfm, ~0);
 
buf = kzalloc(keylen + QCE_MAX_ALIGN_SIZE, GFP_KERNEL);
@@ -420,13 +404,7 @@ static int qce_ahash_hmac_setkey(struct crypto_ahash *tfm, 
const u8 *key,
sg_init_one(&sg, buf, keylen);
ahash_request_set_crypt(req, &sg, ctx->authkey, keylen);
 
-   ret = crypto_ahash_digest(req);
-   if (ret == -EINPROGRESS || ret == -EBUSY) {
-   ret = wait_for_completion_interruptible(&result.completion);
-   if (!ret)
-   ret = result.error;
-   }
-
+   ret = crypto_wait_req(crypto_ahash_digest(req), &wait);
if (ret)
crypto_ahash_set_flags(tfm, CRYPTO_TFM_RES_BAD_KEY_LEN);
 
-- 
2.1.4

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

[PATCH v8 20/20] crypto: adapt api sample to use async. op wait

[Gilad Ben-Yossef]

The code sample is waiting for an async. crypto op completion.
Adapt sample to use the new generic infrastructure to do the same.

This also fixes a possible data coruption bug created by the
use of wait_for_completion_interruptible() without dealing
correctly with an interrupt aborting the wait prior to the
async op finishing.

Signed-off-by: Gilad Ben-Yossef 
---
 Documentation/crypto/api-samples.rst | 52 +++-
 1 file changed, 10 insertions(+), 42 deletions(-)

diff --git a/Documentation/crypto/api-samples.rst 
b/Documentation/crypto/api-samples.rst
index 2531948..006827e 100644
--- a/Documentation/crypto/api-samples.rst
+++ b/Documentation/crypto/api-samples.rst
@@ -7,59 +7,27 @@ Code Example For Symmetric Key Cipher Operation
 ::
 
 
-struct tcrypt_result {
-struct completion completion;
-int err;
-};
-
 /* tie all data structures together */
 struct skcipher_def {
 struct scatterlist sg;
 struct crypto_skcipher *tfm;
 struct skcipher_request *req;
-struct tcrypt_result result;
+struct crypto_wait wait;
 };
 
-/* Callback function */
-static void test_skcipher_cb(struct crypto_async_request *req, int error)
-{
-struct tcrypt_result *result = req->data;
-
-if (error == -EINPROGRESS)
-return;
-result->err = error;
-complete(&result->completion);
-pr_info("Encryption finished successfully\n");
-}
-
 /* Perform cipher operation */
 static unsigned int test_skcipher_encdec(struct skcipher_def *sk,
  int enc)
 {
-int rc = 0;
+int rc;
 
 if (enc)
-rc = crypto_skcipher_encrypt(sk->req);
+rc = crypto_wait_req(crypto_skcipher_encrypt(sk->req), &sk->wait);
 else
-rc = crypto_skcipher_decrypt(sk->req);
-
-switch (rc) {
-case 0:
-break;
-case -EINPROGRESS:
-case -EBUSY:
-rc = wait_for_completion_interruptible(
-&sk->result.completion);
-if (!rc && !sk->result.err) {
-reinit_completion(&sk->result.completion);
-break;
-}
-default:
-pr_info("skcipher encrypt returned with %d result %d\n",
-rc, sk->result.err);
-break;
-}
-init_completion(&sk->result.completion);
+rc = crypto_wait_req(crypto_skcipher_decrypt(sk->req), &sk->wait);
+
+   if (rc)
+   pr_info("skcipher encrypt returned with result %d\n", rc);
 
 return rc;
 }
@@ -89,8 +57,8 @@ Code Example For Symmetric Key Cipher Operation
 }
 
 skcipher_request_set_callback(req, CRYPTO_TFM_REQ_MAY_BACKLOG,
-  test_skcipher_cb,
-  &sk.result);
+  crypto_req_done,
+  &sk.wait);
 
 /* AES 256 with random key */
 get_random_bytes(&key, 32);
@@ -122,7 +90,7 @@ Code Example For Symmetric Key Cipher Operation
 /* We encrypt one block */
 sg_init_one(&sk.sg, scratchpad, 16);
 skcipher_request_set_crypt(req, &sk.sg, &sk.sg, 16, ivdata);
-init_completion(&sk.result.completion);
+crypto_init_wait(&sk.wait);
 
 /* encrypt data */
 ret = test_skcipher_encdec(&sk, 1);
-- 
2.1.4

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

[PATCH v8 15/20] ima: move to generic async completion

[Gilad Ben-Yossef]

ima starts several async crypto ops and  waits for their completions.
Move it over to generic code doing the same.

Signed-off-by: Gilad Ben-Yossef 
Acked-by: Mimi Zohar 
---
 security/integrity/ima/ima_crypto.c | 56 +++--
 1 file changed, 17 insertions(+), 39 deletions(-)

diff --git a/security/integrity/ima/ima_crypto.c 
b/security/integrity/ima/ima_crypto.c
index a856d8c..9057b16 100644
--- a/security/integrity/ima/ima_crypto.c
+++ b/security/integrity/ima/ima_crypto.c
@@ -27,11 +27,6 @@
 
 #include "ima.h"
 
-struct ahash_completion {
-   struct completion completion;
-   int err;
-};
-
 /* minimum file size for ahash use */
 static unsigned long ima_ahash_minsize;
 module_param_named(ahash_minsize, ima_ahash_minsize, ulong, 0644);
@@ -196,30 +191,13 @@ static void ima_free_atfm(struct crypto_ahash *tfm)
crypto_free_ahash(tfm);
 }
 
-static void ahash_complete(struct crypto_async_request *req, int err)
+static inline int ahash_wait(int err, struct crypto_wait *wait)
 {
-   struct ahash_completion *res = req->data;
 
-   if (err == -EINPROGRESS)
-   return;
-   res->err = err;
-   complete(&res->completion);
-}
+   err = crypto_wait_req(err, wait);
 
-static int ahash_wait(int err, struct ahash_completion *res)
-{
-   switch (err) {
-   case 0:
-   break;
-   case -EINPROGRESS:
-   case -EBUSY:
-   wait_for_completion(&res->completion);
-   reinit_completion(&res->completion);
-   err = res->err;
-   /* fall through */
-   default:
+   if (err)
pr_crit_ratelimited("ahash calculation failed: err: %d\n", err);
-   }
 
return err;
 }
@@ -233,7 +211,7 @@ static int ima_calc_file_hash_atfm(struct file *file,
int rc, read = 0, rbuf_len, active = 0, ahash_rc = 0;
struct ahash_request *req;
struct scatterlist sg[1];
-   struct ahash_completion res;
+   struct crypto_wait wait;
size_t rbuf_size[2];
 
hash->length = crypto_ahash_digestsize(tfm);
@@ -242,12 +220,12 @@ static int ima_calc_file_hash_atfm(struct file *file,
if (!req)
return -ENOMEM;
 
-   init_completion(&res.completion);
+   crypto_init_wait(&wait);
ahash_request_set_callback(req, CRYPTO_TFM_REQ_MAY_BACKLOG |
   CRYPTO_TFM_REQ_MAY_SLEEP,
-  ahash_complete, &res);
+  crypto_req_done, &wait);
 
-   rc = ahash_wait(crypto_ahash_init(req), &res);
+   rc = ahash_wait(crypto_ahash_init(req), &wait);
if (rc)
goto out1;
 
@@ -288,7 +266,7 @@ static int ima_calc_file_hash_atfm(struct file *file,
 * read/request, wait for the completion of the
 * previous ahash_update() request.
 */
-   rc = ahash_wait(ahash_rc, &res);
+   rc = ahash_wait(ahash_rc, &wait);
if (rc)
goto out3;
}
@@ -304,7 +282,7 @@ static int ima_calc_file_hash_atfm(struct file *file,
 * read/request, wait for the completion of the
 * previous ahash_update() request.
 */
-   rc = ahash_wait(ahash_rc, &res);
+   rc = ahash_wait(ahash_rc, &wait);
if (rc)
goto out3;
}
@@ -318,7 +296,7 @@ static int ima_calc_file_hash_atfm(struct file *file,
active = !active; /* swap buffers, if we use two */
}
/* wait for the last update request to complete */
-   rc = ahash_wait(ahash_rc, &res);
+   rc = ahash_wait(ahash_rc, &wait);
 out3:
if (read)
file->f_mode &= ~FMODE_READ;
@@ -327,7 +305,7 @@ static int ima_calc_file_hash_atfm(struct file *file,
 out2:
if (!rc) {
ahash_request_set_crypt(req, NULL, hash->digest, 0);
-   rc = ahash_wait(crypto_ahash_final(req), &res);
+   rc = ahash_wait(crypto_ahash_final(req), &wait);
}
 out1:
ahash_request_free(req);
@@ -537,7 +515,7 @@ static int calc_buffer_ahash_atfm(const void *buf, loff_t 
len,
 {
struct ahash_request *req;
struct scatterlist sg;
-   struct ahash_completion res;
+   struct crypto_wait wait;
int rc, ahash_rc = 0;
 
hash->length = crypto_ahash_digestsize(tfm);
@@ -546,12 +524,12 @@ static int calc_buffer_ahash_atfm(const void *buf, loff_t 
len,
if (!req)
return -ENOMEM;
 
-   init_completion(&res.completion);
+   crypto_init_wait(&wait);
ahash_request_set_callback(req, CRYPTO_TFM_REQ_MAY_BACKLOG |
   CRYPTO_TFM_REQ_MAY_SLEEP,
-   

[PATCH v8 14/20] cifs: move to generic async completion

[Gilad Ben-Yossef]

cifs starts an async. crypto op and waits for their completion.
Move it over to generic code doing the same.

Signed-off-by: Gilad Ben-Yossef 
Acked-by: Pavel Shilovsky 
---
 fs/cifs/smb2ops.c | 30 --
 1 file changed, 4 insertions(+), 26 deletions(-)

diff --git a/fs/cifs/smb2ops.c b/fs/cifs/smb2ops.c
index fb2934b..982b39d 100644
--- a/fs/cifs/smb2ops.c
+++ b/fs/cifs/smb2ops.c
@@ -2066,22 +2066,6 @@ init_sg(struct smb_rqst *rqst, u8 *sign)
return sg;
 }
 
-struct cifs_crypt_result {
-   int err;
-   struct completion completion;
-};
-
-static void cifs_crypt_complete(struct crypto_async_request *req, int err)
-{
-   struct cifs_crypt_result *res = req->data;
-
-   if (err == -EINPROGRESS)
-   return;
-
-   res->err = err;
-   complete(&res->completion);
-}
-
 static int
 smb2_get_enc_key(struct TCP_Server_Info *server, __u64 ses_id, int enc, u8 
*key)
 {
@@ -2122,12 +2106,10 @@ crypt_message(struct TCP_Server_Info *server, struct 
smb_rqst *rqst, int enc)
struct aead_request *req;
char *iv;
unsigned int iv_len;
-   struct cifs_crypt_result result = {0, };
+   DECLARE_CRYPTO_WAIT(wait);
struct crypto_aead *tfm;
unsigned int crypt_len = le32_to_cpu(tr_hdr->OriginalMessageSize);
 
-   init_completion(&result.completion);
-
rc = smb2_get_enc_key(server, tr_hdr->SessionId, enc, key);
if (rc) {
cifs_dbg(VFS, "%s: Could not get %scryption key\n", __func__,
@@ -2187,14 +2169,10 @@ crypt_message(struct TCP_Server_Info *server, struct 
smb_rqst *rqst, int enc)
aead_request_set_ad(req, assoc_data_len);
 
aead_request_set_callback(req, CRYPTO_TFM_REQ_MAY_BACKLOG,
- cifs_crypt_complete, &result);
+ crypto_req_done, &wait);
 
-   rc = enc ? crypto_aead_encrypt(req) : crypto_aead_decrypt(req);
-
-   if (rc == -EINPROGRESS || rc == -EBUSY) {
-   wait_for_completion(&result.completion);
-   rc = result.err;
-   }
+   rc = crypto_wait_req(enc ? crypto_aead_encrypt(req)
+   : crypto_aead_decrypt(req), &wait);
 
if (!rc && enc)
memcpy(&tr_hdr->Signature, sign, SMB2_SIGNATURE_SIZE);
-- 
2.1.4

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

[PATCH v8 11/20] crypto: move testmgr to generic async completion

[Gilad Ben-Yossef]

testmgr is starting async. crypto ops and waiting for them to complete.
Move it over to generic code doing the same.

This also provides a test of the generic crypto async. wait code.

Signed-off-by: Gilad Ben-Yossef 
---
 crypto/testmgr.c | 204 ++-
 1 file changed, 66 insertions(+), 138 deletions(-)

diff --git a/crypto/testmgr.c b/crypto/testmgr.c
index 8a124d3..af968f4 100644
--- a/crypto/testmgr.c
+++ b/crypto/testmgr.c
@@ -76,11 +76,6 @@ int alg_test(const char *driver, const char *alg, u32 type, 
u32 mask)
 #define ENCRYPT 1
 #define DECRYPT 0
 
-struct tcrypt_result {
-   struct completion completion;
-   int err;
-};
-
 struct aead_test_suite {
struct {
const struct aead_testvec *vecs;
@@ -155,17 +150,6 @@ static void hexdump(unsigned char *buf, unsigned int len)
buf, len, false);
 }
 
-static void tcrypt_complete(struct crypto_async_request *req, int err)
-{
-   struct tcrypt_result *res = req->data;
-
-   if (err == -EINPROGRESS)
-   return;
-
-   res->err = err;
-   complete(&res->completion);
-}
-
 static int testmgr_alloc_buf(char *buf[XBUFSIZE])
 {
int i;
@@ -193,20 +177,10 @@ static void testmgr_free_buf(char *buf[XBUFSIZE])
free_page((unsigned long)buf[i]);
 }
 
-static int wait_async_op(struct tcrypt_result *tr, int ret)
-{
-   if (ret == -EINPROGRESS || ret == -EBUSY) {
-   wait_for_completion(&tr->completion);
-   reinit_completion(&tr->completion);
-   ret = tr->err;
-   }
-   return ret;
-}
-
 static int ahash_partial_update(struct ahash_request **preq,
struct crypto_ahash *tfm, const struct hash_testvec *template,
void *hash_buff, int k, int temp, struct scatterlist *sg,
-   const char *algo, char *result, struct tcrypt_result *tresult)
+   const char *algo, char *result, struct crypto_wait *wait)
 {
char *state;
struct ahash_request *req;
@@ -236,7 +210,7 @@ static int ahash_partial_update(struct ahash_request **preq,
}
ahash_request_set_callback(req,
CRYPTO_TFM_REQ_MAY_BACKLOG,
-   tcrypt_complete, tresult);
+   crypto_req_done, wait);
 
memcpy(hash_buff, template->plaintext + temp,
template->tap[k]);
@@ -247,7 +221,7 @@ static int ahash_partial_update(struct ahash_request **preq,
pr_err("alg: hash: Failed to import() for %s\n", algo);
goto out;
}
-   ret = wait_async_op(tresult, crypto_ahash_update(req));
+   ret = crypto_wait_req(crypto_ahash_update(req), wait);
if (ret)
goto out;
*preq = req;
@@ -272,7 +246,7 @@ static int __test_hash(struct crypto_ahash *tfm,
char *result;
char *key;
struct ahash_request *req;
-   struct tcrypt_result tresult;
+   struct crypto_wait wait;
void *hash_buff;
char *xbuf[XBUFSIZE];
int ret = -ENOMEM;
@@ -286,7 +260,7 @@ static int __test_hash(struct crypto_ahash *tfm,
if (testmgr_alloc_buf(xbuf))
goto out_nobuf;
 
-   init_completion(&tresult.completion);
+   crypto_init_wait(&wait);
 
req = ahash_request_alloc(tfm, GFP_KERNEL);
if (!req) {
@@ -295,7 +269,7 @@ static int __test_hash(struct crypto_ahash *tfm,
goto out_noreq;
}
ahash_request_set_callback(req, CRYPTO_TFM_REQ_MAY_BACKLOG,
-  tcrypt_complete, &tresult);
+  crypto_req_done, &wait);
 
j = 0;
for (i = 0; i < tcount; i++) {
@@ -335,26 +309,26 @@ static int __test_hash(struct crypto_ahash *tfm,
 
ahash_request_set_crypt(req, sg, result, template[i].psize);
if (use_digest) {
-   ret = wait_async_op(&tresult, crypto_ahash_digest(req));
+   ret = crypto_wait_req(crypto_ahash_digest(req), &wait);
if (ret) {
pr_err("alg: hash: digest failed on test %d "
   "for %s: ret=%d\n", j, algo, -ret);
goto out;
}
} else {
-   ret = wait_async_op(&tresult, crypto_ahash_init(req));
+   ret = crypto_wait_req(crypto_ahash_init(req), &wait);
if (ret) {
pr_err("alg: hash: init failed on test %d "
   "for %s: ret=%d\n", j, algo, -ret);
goto out;
}
-   ret = wait_async_op(&tresult, crypto_ahash_update(req));
+   ret = crypto_wait_req(crypto_ahash_update(req), &wait);
if (ret) {
pr_err("alg: has

[PATCH v8 09/20] crypto: move drbg to generic async completion

[Gilad Ben-Yossef]

DRBG is starting an async. crypto op and waiting for it complete.
Move it over to generic code doing the same.

The code now also passes CRYPTO_TFM_REQ_MAY_SLEEP flag indicating
crypto request memory allocation may use GFP_KERNEL which should
be perfectly fine as the code is obviously sleeping for the
completion of the request any way.

Signed-off-by: Gilad Ben-Yossef 
---
 crypto/drbg.c | 36 +---
 include/crypto/drbg.h |  3 +--
 2 files changed, 10 insertions(+), 29 deletions(-)

diff --git a/crypto/drbg.c b/crypto/drbg.c
index 633a88e..c522251 100644
--- a/crypto/drbg.c
+++ b/crypto/drbg.c
@@ -1651,16 +1651,6 @@ static int drbg_fini_sym_kernel(struct drbg_state *drbg)
return 0;
 }
 
-static void drbg_skcipher_cb(struct crypto_async_request *req, int error)
-{
-   struct drbg_state *drbg = req->data;
-
-   if (error == -EINPROGRESS)
-   return;
-   drbg->ctr_async_err = error;
-   complete(&drbg->ctr_completion);
-}
-
 static int drbg_init_sym_kernel(struct drbg_state *drbg)
 {
struct crypto_cipher *tfm;
@@ -1691,7 +1681,7 @@ static int drbg_init_sym_kernel(struct drbg_state *drbg)
return PTR_ERR(sk_tfm);
}
drbg->ctr_handle = sk_tfm;
-   init_completion(&drbg->ctr_completion);
+   crypto_init_wait(&drbg->ctr_wait);
 
req = skcipher_request_alloc(sk_tfm, GFP_KERNEL);
if (!req) {
@@ -1700,8 +1690,9 @@ static int drbg_init_sym_kernel(struct drbg_state *drbg)
return -ENOMEM;
}
drbg->ctr_req = req;
-   skcipher_request_set_callback(req, CRYPTO_TFM_REQ_MAY_BACKLOG,
-   drbg_skcipher_cb, drbg);
+   skcipher_request_set_callback(req, CRYPTO_TFM_REQ_MAY_BACKLOG |
+   CRYPTO_TFM_REQ_MAY_SLEEP,
+   crypto_req_done, &drbg->ctr_wait);
 
alignmask = crypto_skcipher_alignmask(sk_tfm);
drbg->ctr_null_value_buf = kzalloc(DRBG_CTR_NULL_LEN + alignmask,
@@ -1762,21 +1753,12 @@ static int drbg_kcapi_sym_ctr(struct drbg_state *drbg,
/* Output buffer may not be valid for SGL, use scratchpad */
skcipher_request_set_crypt(drbg->ctr_req, &sg_in, &sg_out,
   cryptlen, drbg->V);
-   ret = crypto_skcipher_encrypt(drbg->ctr_req);
-   switch (ret) {
-   case 0:
-   break;
-   case -EINPROGRESS:
-   case -EBUSY:
-   wait_for_completion(&drbg->ctr_completion);
-   if (!drbg->ctr_async_err) {
-   reinit_completion(&drbg->ctr_completion);
-   break;
-   }
-   default:
+   ret = crypto_wait_req(crypto_skcipher_encrypt(drbg->ctr_req),
+   &drbg->ctr_wait);
+   if (ret)
goto out;
-   }
-   init_completion(&drbg->ctr_completion);
+
+   crypto_init_wait(&drbg->ctr_wait);
 
memcpy(outbuf, drbg->outscratchpad, cryptlen);
 
diff --git a/include/crypto/drbg.h b/include/crypto/drbg.h
index 22f884c..8f94110 100644
--- a/include/crypto/drbg.h
+++ b/include/crypto/drbg.h
@@ -126,8 +126,7 @@ struct drbg_state {
__u8 *ctr_null_value;   /* CTR mode aligned zero buf */
__u8 *outscratchpadbuf; /* CTR mode output scratchpad */
 __u8 *outscratchpad;   /* CTR mode aligned outbuf */
-   struct completion ctr_completion;   /* CTR mode async handler */
-   int ctr_async_err;  /* CTR mode async error */
+   struct crypto_wait ctr_wait;/* CTR mode async wait obj */
 
bool seeded;/* DRBG fully seeded? */
bool pr;/* Prediction resistance enabled? */
-- 
2.1.4

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

[PATCH v8 10/20] crypto: move gcm to generic async completion

[Gilad Ben-Yossef]

gcm is starting an async. crypto op and waiting for it complete.
Move it over to generic code doing the same.

Signed-off-by: Gilad Ben-Yossef 
---
 crypto/gcm.c | 32 ++--
 1 file changed, 6 insertions(+), 26 deletions(-)

diff --git a/crypto/gcm.c b/crypto/gcm.c
index 3841b5e..fb923a5 100644
--- a/crypto/gcm.c
+++ b/crypto/gcm.c
@@ -16,7 +16,6 @@
 #include 
 #include 
 #include "internal.h"
-#include 
 #include 
 #include 
 #include 
@@ -78,11 +77,6 @@ struct crypto_gcm_req_priv_ctx {
} u;
 };
 
-struct crypto_gcm_setkey_result {
-   int err;
-   struct completion completion;
-};
-
 static struct {
u8 buf[16];
struct scatterlist sg;
@@ -98,17 +92,6 @@ static inline struct crypto_gcm_req_priv_ctx 
*crypto_gcm_reqctx(
return (void *)PTR_ALIGN((u8 *)aead_request_ctx(req), align + 1);
 }
 
-static void crypto_gcm_setkey_done(struct crypto_async_request *req, int err)
-{
-   struct crypto_gcm_setkey_result *result = req->data;
-
-   if (err == -EINPROGRESS)
-   return;
-
-   result->err = err;
-   complete(&result->completion);
-}
-
 static int crypto_gcm_setkey(struct crypto_aead *aead, const u8 *key,
 unsigned int keylen)
 {
@@ -119,7 +102,7 @@ static int crypto_gcm_setkey(struct crypto_aead *aead, 
const u8 *key,
be128 hash;
u8 iv[16];
 
-   struct crypto_gcm_setkey_result result;
+   struct crypto_wait wait;
 
struct scatterlist sg[1];
struct skcipher_request req;
@@ -140,21 +123,18 @@ static int crypto_gcm_setkey(struct crypto_aead *aead, 
const u8 *key,
if (!data)
return -ENOMEM;
 
-   init_completion(&data->result.completion);
+   crypto_init_wait(&data->wait);
sg_init_one(data->sg, &data->hash, sizeof(data->hash));
skcipher_request_set_tfm(&data->req, ctr);
skcipher_request_set_callback(&data->req, CRYPTO_TFM_REQ_MAY_SLEEP |
  CRYPTO_TFM_REQ_MAY_BACKLOG,
- crypto_gcm_setkey_done,
- &data->result);
+ crypto_req_done,
+ &data->wait);
skcipher_request_set_crypt(&data->req, data->sg, data->sg,
   sizeof(data->hash), data->iv);
 
-   err = crypto_skcipher_encrypt(&data->req);
-   if (err == -EINPROGRESS || err == -EBUSY) {
-   wait_for_completion(&data->result.completion);
-   err = data->result.err;
-   }
+   err = crypto_wait_req(crypto_skcipher_encrypt(&data->req),
+   &data->wait);
 
if (err)
goto out;
-- 
2.1.4

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

[PATCH v8 04/20] crypto: remove redundant backlog checks on EBUSY

[Gilad Ben-Yossef]

Now that -EBUSY return code only indicates backlog queueing
we can safely remove the now redundant check for the
CRYPTO_TFM_REQ_MAY_BACKLOG flag when -EBUSY is returned.

Signed-off-by: Gilad Ben-Yossef 
---
 crypto/ahash.c| 12 +++-
 crypto/cts.c  |  6 ++
 crypto/lrw.c  |  8 ++--
 crypto/rsa-pkcs1pad.c | 16 
 crypto/xts.c  |  8 ++--
 5 files changed, 13 insertions(+), 37 deletions(-)

diff --git a/crypto/ahash.c b/crypto/ahash.c
index 5e8666e..3a35d67 100644
--- a/crypto/ahash.c
+++ b/crypto/ahash.c
@@ -334,9 +334,7 @@ static int ahash_op_unaligned(struct ahash_request *req,
return err;
 
err = op(req);
-   if (err == -EINPROGRESS ||
-   (err == -EBUSY && (ahash_request_flags(req) &
-  CRYPTO_TFM_REQ_MAY_BACKLOG)))
+   if (err == -EINPROGRESS || err == -EBUSY)
return err;
 
ahash_restore_req(req, err);
@@ -394,9 +392,7 @@ static int ahash_def_finup_finish1(struct ahash_request 
*req, int err)
req->base.complete = ahash_def_finup_done2;
 
err = crypto_ahash_reqtfm(req)->final(req);
-   if (err == -EINPROGRESS ||
-   (err == -EBUSY && (ahash_request_flags(req) &
-  CRYPTO_TFM_REQ_MAY_BACKLOG)))
+   if (err == -EINPROGRESS || err == -EBUSY)
return err;
 
 out:
@@ -432,9 +428,7 @@ static int ahash_def_finup(struct ahash_request *req)
return err;
 
err = tfm->update(req);
-   if (err == -EINPROGRESS ||
-   (err == -EBUSY && (ahash_request_flags(req) &
-  CRYPTO_TFM_REQ_MAY_BACKLOG)))
+   if (err == -EINPROGRESS || err == -EBUSY)
return err;
 
return ahash_def_finup_finish1(req, err);
diff --git a/crypto/cts.c b/crypto/cts.c
index 243f591..4773c18 100644
--- a/crypto/cts.c
+++ b/crypto/cts.c
@@ -136,8 +136,7 @@ static void crypto_cts_encrypt_done(struct 
crypto_async_request *areq, int err)
goto out;
 
err = cts_cbc_encrypt(req);
-   if (err == -EINPROGRESS ||
-   (err == -EBUSY && req->base.flags & CRYPTO_TFM_REQ_MAY_BACKLOG))
+   if (err == -EINPROGRESS || err == -EBUSY)
return;
 
 out:
@@ -229,8 +228,7 @@ static void crypto_cts_decrypt_done(struct 
crypto_async_request *areq, int err)
goto out;
 
err = cts_cbc_decrypt(req);
-   if (err == -EINPROGRESS ||
-   (err == -EBUSY && req->base.flags & CRYPTO_TFM_REQ_MAY_BACKLOG))
+   if (err == -EINPROGRESS || err == -EBUSY)
return;
 
 out:
diff --git a/crypto/lrw.c b/crypto/lrw.c
index a8bfae4..695cea9 100644
--- a/crypto/lrw.c
+++ b/crypto/lrw.c
@@ -328,9 +328,7 @@ static int do_encrypt(struct skcipher_request *req, int err)
  crypto_skcipher_encrypt(subreq) ?:
  post_crypt(req);
 
-   if (err == -EINPROGRESS ||
-   (err == -EBUSY &&
-req->base.flags & CRYPTO_TFM_REQ_MAY_BACKLOG))
+   if (err == -EINPROGRESS || err == -EBUSY)
return err;
}
 
@@ -380,9 +378,7 @@ static int do_decrypt(struct skcipher_request *req, int err)
  crypto_skcipher_decrypt(subreq) ?:
  post_crypt(req);
 
-   if (err == -EINPROGRESS ||
-   (err == -EBUSY &&
-req->base.flags & CRYPTO_TFM_REQ_MAY_BACKLOG))
+   if (err == -EINPROGRESS || err == -EBUSY)
return err;
}
 
diff --git a/crypto/rsa-pkcs1pad.c b/crypto/rsa-pkcs1pad.c
index 407c64b..2908f93 100644
--- a/crypto/rsa-pkcs1pad.c
+++ b/crypto/rsa-pkcs1pad.c
@@ -279,9 +279,7 @@ static int pkcs1pad_encrypt(struct akcipher_request *req)
   req->dst, ctx->key_size - 1, req->dst_len);
 
err = crypto_akcipher_encrypt(&req_ctx->child_req);
-   if (err != -EINPROGRESS &&
-   (err != -EBUSY ||
-!(req->base.flags & CRYPTO_TFM_REQ_MAY_BACKLOG)))
+   if (err != -EINPROGRESS && err != -EBUSY)
return pkcs1pad_encrypt_sign_complete(req, err);
 
return err;
@@ -383,9 +381,7 @@ static int pkcs1pad_decrypt(struct akcipher_request *req)
   ctx->key_size);
 
err = crypto_akcipher_decrypt(&req_ctx->child_req);
-   if (err != -EINPROGRESS &&
-   (err != -EBUSY ||
-!(req->base.flags & CRYPTO_TFM_REQ_MAY_BACKLOG)))
+   if (err != -EINPROGRESS && err != -EBUSY)
return pkcs1pad_decrypt_complete(req, err);
 
return err;
@@ -440,9 +436,7 @@ static int pkcs1pad_sign(struct akcipher_request *req)
   req->dst, ctx->key_size - 1, req->dst_len);
 
err = crypto_akcipher_sign(&req_ctx->child_req);
-   if (err != -EI

Re: [v7 1/5] mm, oom: refactor the oom_kill_process() function

[Michal Hocko]

On Mon 04-09-17 15:21:04, Roman Gushchin wrote:
> The oom_kill_process() function consists of two logical parts:
> the first one is responsible for considering task's children as
> a potential victim and printing the debug information.
> The second half is responsible for sending SIGKILL to all
> tasks sharing the mm struct with the given victim.
> 
> This commit splits the oom_kill_process() function with
> an intention to re-use the the second half: __oom_kill_process().
> 
> The cgroup-aware OOM killer will kill multiple tasks
> belonging to the victim cgroup. We don't need to print
> the debug information for the each task, as well as play
> with task selection (considering task's children),
> so we can't use the existing oom_kill_process().
> 
> Signed-off-by: Roman Gushchin 
> Cc: Michal Hocko 
> Cc: Vladimir Davydov 
> Cc: Johannes Weiner 
> Cc: Tetsuo Handa 
> Cc: David Rientjes 
> Cc: Andrew Morton 
> Cc: Tejun Heo 
> Cc: kernel-t...@fb.com
> Cc: cgro...@vger.kernel.org
> Cc: linux-doc@vger.kernel.org
> Cc: linux-ker...@vger.kernel.org
> Cc: linux...@kvack.org

Looks good to me
Acked-by: Michal Hocko 

> ---
>  mm/oom_kill.c | 123 
> +++---
>  1 file changed, 65 insertions(+), 58 deletions(-)
> 
> diff --git a/mm/oom_kill.c b/mm/oom_kill.c
> index 99736e026712..f061b627092c 100644
> --- a/mm/oom_kill.c
> +++ b/mm/oom_kill.c
> @@ -804,68 +804,12 @@ static bool task_will_free_mem(struct task_struct *task)
>   return ret;
>  }
>  
> -static void oom_kill_process(struct oom_control *oc, const char *message)
> +static void __oom_kill_process(struct task_struct *victim)
>  {
> - struct task_struct *p = oc->chosen;
> - unsigned int points = oc->chosen_points;
> - struct task_struct *victim = p;
> - struct task_struct *child;
> - struct task_struct *t;
> + struct task_struct *p;
>   struct mm_struct *mm;
> - unsigned int victim_points = 0;
> - static DEFINE_RATELIMIT_STATE(oom_rs, DEFAULT_RATELIMIT_INTERVAL,
> -   DEFAULT_RATELIMIT_BURST);
>   bool can_oom_reap = true;
>  
> - /*
> -  * If the task is already exiting, don't alarm the sysadmin or kill
> -  * its children or threads, just give it access to memory reserves
> -  * so it can die quickly
> -  */
> - task_lock(p);
> - if (task_will_free_mem(p)) {
> - mark_oom_victim(p);
> - wake_oom_reaper(p);
> - task_unlock(p);
> - put_task_struct(p);
> - return;
> - }
> - task_unlock(p);
> -
> - if (__ratelimit(&oom_rs))
> - dump_header(oc, p);
> -
> - pr_err("%s: Kill process %d (%s) score %u or sacrifice child\n",
> - message, task_pid_nr(p), p->comm, points);
> -
> - /*
> -  * If any of p's children has a different mm and is eligible for kill,
> -  * the one with the highest oom_badness() score is sacrificed for its
> -  * parent.  This attempts to lose the minimal amount of work done while
> -  * still freeing memory.
> -  */
> - read_lock(&tasklist_lock);
> - for_each_thread(p, t) {
> - list_for_each_entry(child, &t->children, sibling) {
> - unsigned int child_points;
> -
> - if (process_shares_mm(child, p->mm))
> - continue;
> - /*
> -  * oom_badness() returns 0 if the thread is unkillable
> -  */
> - child_points = oom_badness(child,
> - oc->memcg, oc->nodemask, oc->totalpages);
> - if (child_points > victim_points) {
> - put_task_struct(victim);
> - victim = child;
> - victim_points = child_points;
> - get_task_struct(victim);
> - }
> - }
> - }
> - read_unlock(&tasklist_lock);
> -
>   p = find_lock_task_mm(victim);
>   if (!p) {
>   put_task_struct(victim);
> @@ -939,6 +883,69 @@ static void oom_kill_process(struct oom_control *oc, 
> const char *message)
>  }
>  #undef K
>  
> +static void oom_kill_process(struct oom_control *oc, const char *message)
> +{
> + struct task_struct *p = oc->chosen;
> + unsigned int points = oc->chosen_points;
> + struct task_struct *victim = p;
> + struct task_struct *child;
> + struct task_struct *t;
> + unsigned int victim_points = 0;
> + static DEFINE_RATELIMIT_STATE(oom_rs, DEFAULT_RATELIMIT_INTERVAL,
> +   DEFAULT_RATELIMIT_BURST);
> +
> + /*
> +  * If the task is already exiting, don't alarm the sysadmin or kill
> +  * its children or threads, just give it access to memory reserves
> +  * so it can die quickly
> +  */
> + task_lock(p);
> + if (task_will_free

Re: [v7 5/5] mm, oom: cgroup v2 mount option to disable cgroup-aware OOM killer

[Michal Hocko]

I will go and check patch 2 more deeply but this is something that I
wanted to sort out first.

On Mon 04-09-17 15:21:08, Roman Gushchin wrote:
> Introducing of cgroup-aware OOM killer changes the victim selection
> algorithm used by default: instead of picking the largest process,
> it will pick the largest memcg and then the largest process inside.
> 
> This affects only cgroup v2 users.
> 
> To provide a way to use cgroups v2 if the old OOM victim selection
> algorithm is preferred for some reason, the nogroupoom mount option
> is added.
> 
> If set, the OOM selection is performed in a "traditional" per-process
> way. Both oom_priority and oom_group memcg knobs are ignored.

Why is this an opt out rather than opt-in? IMHO the original oom logic
should be preserved by default and specific workloads should opt in for
the cgroup aware logic. Changing the global behavior depending on
whether cgroup v2 interface is in use is more than unexpected and IMHO
wrong approach to take. I think we should instead go with 
oom_strategy=[alloc_task,biggest_task,cgroup]

we currently have alloc_task (via sysctl_oom_kill_allocating_task) and
biggest_task which is the default. You are adding cgroup and the more I
think about the more I agree that it doesn't really make sense to try to
fit thew new semantic into the existing one (compare tasks to kill-all
memcgs). Just introduce a new strategy and define a new semantic from
scratch. Memcg priority and kill-all are a natural extension of this new
strategy. This will make the life easier and easier to understand by
users.

Does that make sense to you?

> Signed-off-by: Roman Gushchin 
> Cc: Michal Hocko 
> Cc: Vladimir Davydov 
> Cc: Johannes Weiner 
> Cc: Tetsuo Handa 
> Cc: David Rientjes 
> Cc: Andrew Morton 
> Cc: Tejun Heo 
> Cc: kernel-t...@fb.com
> Cc: cgro...@vger.kernel.org
> Cc: linux-doc@vger.kernel.org
> Cc: linux-ker...@vger.kernel.org
> Cc: linux...@kvack.org
> ---
>  Documentation/admin-guide/kernel-parameters.txt | 1 +
>  mm/memcontrol.c | 8 
>  2 files changed, 9 insertions(+)
> 
> diff --git a/Documentation/admin-guide/kernel-parameters.txt 
> b/Documentation/admin-guide/kernel-parameters.txt
> index 28f1a0f84456..07891f1030aa 100644
> --- a/Documentation/admin-guide/kernel-parameters.txt
> +++ b/Documentation/admin-guide/kernel-parameters.txt
> @@ -489,6 +489,7 @@
>   Format: 
>   nosocket -- Disable socket memory accounting.
>   nokmem -- Disable kernel memory accounting.
> + nogroupoom -- Disable cgroup-aware OOM killer.
>  
>   checkreqprot[SELINUX] Set initial checkreqprot flag value.
>   Format: { "0" | "1" }
> diff --git a/mm/memcontrol.c b/mm/memcontrol.c
> index d7dd293897ca..6a8235dc41f6 100644
> --- a/mm/memcontrol.c
> +++ b/mm/memcontrol.c
> @@ -87,6 +87,9 @@ static bool cgroup_memory_nosocket;
>  /* Kernel memory accounting disabled? */
>  static bool cgroup_memory_nokmem;
>  
> +/* Cgroup-aware OOM  disabled? */
> +static bool cgroup_memory_nogroupoom;
> +
>  /* Whether the swap controller is active */
>  #ifdef CONFIG_MEMCG_SWAP
>  int do_swap_account __read_mostly;
> @@ -2822,6 +2825,9 @@ bool mem_cgroup_select_oom_victim(struct oom_control 
> *oc)
>   if (mem_cgroup_disabled())
>   return false;
>  
> + if (cgroup_memory_nogroupoom)
> + return false;
> +
>   if (!cgroup_subsys_on_dfl(memory_cgrp_subsys))
>   return false;
>  
> @@ -6188,6 +6194,8 @@ static int __init cgroup_memory(char *s)
>   cgroup_memory_nosocket = true;
>   if (!strcmp(token, "nokmem"))
>   cgroup_memory_nokmem = true;
> + if (!strcmp(token, "nogroupoom"))
> + cgroup_memory_nogroupoom = true;
>   }
>   return 0;
>  }
> -- 
> 2.13.5

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

Re: [v7 5/5] mm, oom: cgroup v2 mount option to disable cgroup-aware OOM killer

[Roman Gushchin]

On Tue, Sep 05, 2017 at 03:44:12PM +0200, Michal Hocko wrote:
> I will go and check patch 2 more deeply but this is something that I
> wanted to sort out first.
> 
> On Mon 04-09-17 15:21:08, Roman Gushchin wrote:
> > Introducing of cgroup-aware OOM killer changes the victim selection
> > algorithm used by default: instead of picking the largest process,
> > it will pick the largest memcg and then the largest process inside.
> > 
> > This affects only cgroup v2 users.
> > 
> > To provide a way to use cgroups v2 if the old OOM victim selection
> > algorithm is preferred for some reason, the nogroupoom mount option
> > is added.
> > 
> > If set, the OOM selection is performed in a "traditional" per-process
> > way. Both oom_priority and oom_group memcg knobs are ignored.
> 
> Why is this an opt out rather than opt-in? IMHO the original oom logic
> should be preserved by default and specific workloads should opt in for
> the cgroup aware logic. Changing the global behavior depending on
> whether cgroup v2 interface is in use is more than unexpected and IMHO
> wrong approach to take. I think we should instead go with 
> oom_strategy=[alloc_task,biggest_task,cgroup]
> 
> we currently have alloc_task (via sysctl_oom_kill_allocating_task) and
> biggest_task which is the default. You are adding cgroup and the more I
> think about the more I agree that it doesn't really make sense to try to
> fit thew new semantic into the existing one (compare tasks to kill-all
> memcgs). Just introduce a new strategy and define a new semantic from
> scratch. Memcg priority and kill-all are a natural extension of this new
> strategy. This will make the life easier and easier to understand by
> users.
> 
> Does that make sense to you?

Absolutely.

The only thing: I'm not sure that we have to preserve the existing logic
as default option. For most users (except few very specific usecases),
it should be at least as good, as the existing one.

Making it opt-in means that corresponding code will be executed only
by few users, who cares. Then we should probably hide corresponding
cgroup interface (oom_group and oom_priority knobs) by default,
and it feels as unnecessary complication and is overall against
cgroup v2 interface design.

> I think we should instead go with
> oom_strategy=[alloc_task,biggest_task,cgroup]

It would be a really nice interface; although I've no idea how to implement it:
"alloc_task" is an existing sysctl, which we have to preserve;
while "cgroup" depends on cgroup v2.

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

Re: [v7 2/5] mm, oom: cgroup-aware OOM killer

[Michal Hocko]

On Mon 04-09-17 15:21:05, Roman Gushchin wrote:
[...]
> diff --git a/mm/memcontrol.c b/mm/memcontrol.c
> index a69d23082abf..97813c56163b 100644
> --- a/mm/memcontrol.c
> +++ b/mm/memcontrol.c
> @@ -2649,6 +2649,213 @@ static inline bool memcg_has_children(struct 
> mem_cgroup *memcg)
>   return ret;
>  }
>  
> +static long memcg_oom_badness(struct mem_cgroup *memcg,
> +   const nodemask_t *nodemask)
> +{
> + long points = 0;
> + int nid;
> + pg_data_t *pgdat;
> +
> + for_each_node_state(nid, N_MEMORY) {
> + if (nodemask && !node_isset(nid, *nodemask))
> + continue;
> +
> + points += mem_cgroup_node_nr_lru_pages(memcg, nid,
> + LRU_ALL_ANON | BIT(LRU_UNEVICTABLE));

Why don't you consider file LRUs here? What if there is a lot of page
cache which is not reclaimed because it is protected by memcg->low.
Should we hide that from the OOM killer?

[...]
> +static void select_victim_memcg(struct mem_cgroup *root, struct oom_control 
> *oc)
> +{
> + struct mem_cgroup *iter, *parent;
> +
> + for_each_mem_cgroup_tree(iter, root) {
> + if (memcg_has_children(iter)) {
> + iter->oom_score = 0;
> + continue;
> + }

Do we really need this check? If it is a mere optimization then
we should probably check for tasks in the memcg rather than
descendant. More on that below.

> +
> + iter->oom_score = oom_evaluate_memcg(iter, oc->nodemask);
> +
> + /*
> +  * Ignore empty and non-eligible memory cgroups.
> +  */
> + if (iter->oom_score == 0)
> + continue;
> +
> + /*
> +  * If there are inflight OOM victims, we don't need to look
> +  * further for new victims.
> +  */
> + if (iter->oom_score == -1) {
> + oc->chosen_memcg = INFLIGHT_VICTIM;
> + mem_cgroup_iter_break(root, iter);
> + return;
> + }
> +
> + for (parent = parent_mem_cgroup(iter); parent && parent != root;
> +  parent = parent_mem_cgroup(parent))
> + parent->oom_score += iter->oom_score;

Hmm. The changelog says "By default, it will look for the biggest leaf
cgroup, and kill the largest task inside." But you are accumulating
oom_score up the hierarchy and so parents will have higher score than
the layer of their children and the larger the sub-hierarchy the more
biased it will become. Say you have
root
 /\
/  \
   AD
  / \
 B   C

B (5), C(15) thus A(20) and D(20). Unless I am missing something we are
going to go down A path and then chose C even though D is the largest
leaf group, right?

> + }
> +
> + for (;;) {
> + struct cgroup_subsys_state *css;
> + struct mem_cgroup *memcg = NULL;
> + long score = LONG_MIN;
> +
> + css_for_each_child(css, &root->css) {
> + struct mem_cgroup *iter = mem_cgroup_from_css(css);
> +
> + /*
> +  * Ignore empty and non-eligible memory cgroups.
> +  */
> + if (iter->oom_score == 0)
> + continue;
> +
> + if (iter->oom_score > score) {
> + memcg = iter;
> + score = iter->oom_score;
> + }
> + }
> +
> + if (!memcg) {
> + if (oc->memcg && root == oc->memcg) {
> + oc->chosen_memcg = oc->memcg;
> + css_get(&oc->chosen_memcg->css);
> + oc->chosen_points = oc->memcg->oom_score;
> + }
> + break;
> + }
> +
> + if (memcg->oom_group || !memcg_has_children(memcg)) {
> + oc->chosen_memcg = memcg;
> + css_get(&oc->chosen_memcg->css);
> + oc->chosen_points = score;
> + break;
> + }
> +
> + root = memcg;
> + }
> +}
> +
[...]
> + /*
> +  * For system-wide OOMs we should consider tasks in the root cgroup
> +  * with oom_score larger than oc->chosen_points.
> +  */
> + if (!oc->memcg) {
> + select_victim_root_cgroup_task(oc);

I do not understand why do we have to handle root cgroup specially here.
select_victim_memcg already iterates all memcgs in the oom hierarchy
(including root) so if the root memcg is the largest one then we
should simply consider it no? You are skipping root there because of
memcg_has_children but I suspect this and the whole accumulate up the
hierarchy approach just makes the whole thing more complex than necessary. With
"tasks only in leafs" cgroup pol

Re: [v7 5/5] mm, oom: cgroup v2 mount option to disable cgroup-aware OOM killer

[Michal Hocko]

On Tue 05-09-17 15:30:21, Roman Gushchin wrote:
> On Tue, Sep 05, 2017 at 03:44:12PM +0200, Michal Hocko wrote:
[...]
> > Why is this an opt out rather than opt-in? IMHO the original oom logic
> > should be preserved by default and specific workloads should opt in for
> > the cgroup aware logic. Changing the global behavior depending on
> > whether cgroup v2 interface is in use is more than unexpected and IMHO
> > wrong approach to take. I think we should instead go with 
> > oom_strategy=[alloc_task,biggest_task,cgroup]
> > 
> > we currently have alloc_task (via sysctl_oom_kill_allocating_task) and
> > biggest_task which is the default. You are adding cgroup and the more I
> > think about the more I agree that it doesn't really make sense to try to
> > fit thew new semantic into the existing one (compare tasks to kill-all
> > memcgs). Just introduce a new strategy and define a new semantic from
> > scratch. Memcg priority and kill-all are a natural extension of this new
> > strategy. This will make the life easier and easier to understand by
> > users.
> > 
> > Does that make sense to you?
> 
> Absolutely.
> 
> The only thing: I'm not sure that we have to preserve the existing logic
> as default option. For most users (except few very specific usecases),
> it should be at least as good, as the existing one.

But this is really an unexpected change. Users even might not know that
they are using cgroup v2 and memcg is in use.

> Making it opt-in means that corresponding code will be executed only
> by few users, who cares.

Yeah, which is the way we should introduce new features no?

> Then we should probably hide corresponding
> cgroup interface (oom_group and oom_priority knobs) by default,
> and it feels as unnecessary complication and is overall against
> cgroup v2 interface design.

Why. If we care enough, we could simply return EINVAL when those knobs
are written while the corresponding strategy is not used.

> > I think we should instead go with
> > oom_strategy=[alloc_task,biggest_task,cgroup]
> 
> It would be a really nice interface; although I've no idea how to implement 
> it:
> "alloc_task" is an existing sysctl, which we have to preserve;

I would argue that we should simply deprecate and later drop the sysctl.
I _strongly_ suspect anybody is using this. If yes it is not that hard
to change the kernel command like rather than select the sysctl. The
deprecation process would be
- warn when somebody writes to the sysctl and check both boot
  and sysctl values
[ wait some time ]
- keep the sysctl but return EINVAL
[ wait some time ]
- remove the sysctl

> while "cgroup" depends on cgroup v2.

Which is not a big deal either. Simply fall back to default if there are
no cgroup v2. The implementation would have essentially the same effect
because there won't be any kill-all cgroups and so we will select the
largest task.
-- 
Michal Hocko
SUSE Labs
--
To unsubscribe from this list: send the line "unsubscribe linux-doc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] hwmon: pmbus: Make reg check and clear faults functions return errors

[Guenter Roeck]

On Tue, Sep 05, 2017 at 05:01:32PM +1000, Andrew Jeffery wrote:
> Some functions exposed by pmbus core conflated errors that occurred when
> setting the page to access with errors that occurred when accessing
> registers in a page. In some cases, this caused legitimate errors to be
> hidden under the guise of the register not being supported.
> 

I'll have to look into this. If I recall correctly, the reason for not returning
errors from the "clear faults" command was that some early chips did not support
it, or did not support it on all pages. Those chips would now always fail.

On a higher level, I am not sure if it is a good idea to return an error
from a function intended to clear faults (and nothing else). That seems
counterproductive.

Is this a problem you have actually observed ? If so, what is the chip ?

Thanks,
Guenter

> Signed-off-by: Andrew Jeffery 
> ---
>  Documentation/hwmon/pmbus-core   |  12 ++--
>  drivers/hwmon/pmbus/pmbus.h  |   6 +-
>  drivers/hwmon/pmbus/pmbus_core.c | 115 
> +--
>  3 files changed, 95 insertions(+), 38 deletions(-)
> 
> diff --git a/Documentation/hwmon/pmbus-core b/Documentation/hwmon/pmbus-core
> index 8ed10e9ddfb5..3e9f41bb756f 100644
> --- a/Documentation/hwmon/pmbus-core
> +++ b/Documentation/hwmon/pmbus-core
> @@ -218,17 +218,17 @@ Specifically, it provides the following information.
>This function calls the device specific write_byte function if defined.
>Therefore, it must _not_ be called from that function.
>  
> -  bool pmbus_check_byte_register(struct i2c_client *client, int page, int 
> reg);
> +  int pmbus_check_byte_register(struct i2c_client *client, int page, int 
> reg);
>  
> -  Check if byte register exists. Return true if the register exists, false
> -  otherwise.
> +  Check if byte register exists. Returns 1 if the register exists, 0 if it 
> does
> +  not, and less than zero on an unexpected error.
>This function calls the device specific write_byte function if defined to
>obtain the chip status. Therefore, it must _not_ be called from that 
> function.
>  
> -  bool pmbus_check_word_register(struct i2c_client *client, int page, int 
> reg);
> +  int pmbus_check_word_register(struct i2c_client *client, int page, int 
> reg);
>  
> -  Check if word register exists. Return true if the register exists, false
> -  otherwise.
> +  Check if word register exists. Returns 1 if the register exists, 0 if it 
> does
> +  not, and less than zero on an unexpected error.
>This function calls the device specific write_byte function if defined to
>obtain the chip status. Therefore, it must _not_ be called from that 
> function.
>  
> diff --git a/drivers/hwmon/pmbus/pmbus.h b/drivers/hwmon/pmbus/pmbus.h
> index bfcb13bae34b..c53032a04a6f 100644
> --- a/drivers/hwmon/pmbus/pmbus.h
> +++ b/drivers/hwmon/pmbus/pmbus.h
> @@ -413,9 +413,9 @@ int pmbus_write_byte_data(struct i2c_client *client, int 
> page, u8 reg,
> u8 value);
>  int pmbus_update_byte_data(struct i2c_client *client, int page, u8 reg,
>  u8 mask, u8 value);
> -void pmbus_clear_faults(struct i2c_client *client);
> -bool pmbus_check_byte_register(struct i2c_client *client, int page, int reg);
> -bool pmbus_check_word_register(struct i2c_client *client, int page, int reg);
> +int pmbus_clear_faults(struct i2c_client *client);
> +int pmbus_check_byte_register(struct i2c_client *client, int page, int reg);
> +int pmbus_check_word_register(struct i2c_client *client, int page, int reg);
>  int pmbus_do_probe(struct i2c_client *client, const struct i2c_device_id *id,
>  struct pmbus_driver_info *info);
>  int pmbus_do_remove(struct i2c_client *client);
> diff --git a/drivers/hwmon/pmbus/pmbus_core.c 
> b/drivers/hwmon/pmbus/pmbus_core.c
> index f1eff6b6c798..153700e35431 100644
> --- a/drivers/hwmon/pmbus/pmbus_core.c
> +++ b/drivers/hwmon/pmbus/pmbus_core.c
> @@ -304,18 +304,24 @@ static int _pmbus_read_byte_data(struct i2c_client 
> *client, int page, int reg)
>   return pmbus_read_byte_data(client, page, reg);
>  }
>  
> -static void pmbus_clear_fault_page(struct i2c_client *client, int page)
> +static int pmbus_clear_fault_page(struct i2c_client *client, int page)
>  {
> - _pmbus_write_byte(client, page, PMBUS_CLEAR_FAULTS);
> + return _pmbus_write_byte(client, page, PMBUS_CLEAR_FAULTS);
>  }
>  
> -void pmbus_clear_faults(struct i2c_client *client)
> +int pmbus_clear_faults(struct i2c_client *client)
>  {
>   struct pmbus_data *data = i2c_get_clientdata(client);
> + int rv;
>   int i;
>  
> - for (i = 0; i < data->info->pages; i++)
> - pmbus_clear_fault_page(client, i);
> + for (i = 0; i < data->info->pages; i++) {
> + rv = pmbus_clear_fault_page(client, i);
> + if (rv)
> + return rv;
> + }
> +
> + return 0;
>  }
>  EXPORT_SYMBOL_GPL(pmbus_clear_faults);
>  
> @@ -333,28 +339,45 @@ 

Re: [PATCH v2] fscrypt: add a documentation file for filesystem-level encryption

[Eric Biggers]

Hi Michael,

On Fri, Sep 01, 2017 at 05:12:28PM -0700, Michael Halcrow wrote:
> > +fscrypt is only resistant to side-channel attacks, such as timing or
> > +electromagnetic attacks, to the extent that the underlying Linux
> > +Cryptographic API algorithms are.  If a vulnerable algorithm is used,
> > +such as a table-based implementation of AES, it may be possible for an
> > +attacker to mount a side channel attack against the online system.
> 
> Conservatively, I'd go a step further and say that even if the kernel
> crypto API were to mitigate all side channel attacks, we don't make
> any claims as to whether encryption key material (or data for that
> matter) would be vulnerable from code in fs/crypto or the individual
> filesystems.

If possible, I'd prefer to make a stronger claim, at least for the key material.
We are very careful with how we manage the keys in the kernel, and I'd consider
any timing attack against the keys to be a bug --- and I expect that other
people would too.

Data is a different story though, since ultimately it is used by applications to
actually do something.

> > +After an encryption key has been provided, fscrypt is not designed to
> > +hide the plaintext file contents or filenames from other users on the
> > +same system, regardless of the visibility of the keyring key.
> > +Instead, existing access control mechanisms such as file mode bits,
> > +POSIX ACLs, or SELinux should be used for this purpose.  Also note
> 
> Suggest replacing "SELinux" with "LSMs."  Also do you think we should
> mention namespaces?

We could mention that a filesystem containing encrypted files can be "hidden" by
unmounting it, though that should be obvious.  It's also possible to hide
individual files or directories by bind-mounting stuff over them, though that is
getting somewhat eccentric; usually mode bits, ACLs, etc. would be used instead.

> > +that as long as the encryption keys are *anywhere* in memory, an
> > +online attacker can necessarily compromise them by mounting a physical
> > +attack or by exploiting any kernel security vulnerability which
> > +provides an arbitrary memory read primitive.
> 
> Or hooking into a FireWire port and doing DMA?

That falls under the category of "physical attack".

> > +Currently, fscrypt does not prevent a user from maliciously providing
> > +an incorrect key for another user's existing encrypted files.  A
> > +protection against this is planned.
> 
> How's the review for that patch coming along?

So far you're the only person who has reviewed the patchset in detail.  I've
tried to get more people interested, but it has been difficult.  Perhaps the new
documentation will help people understand the problems that the patchset solves.
Being able to provide the wrong key for another user's encrypted files is a
security vulnerability, so we *have* to fix it eventually.

> > +- Per-file keys strengthen the encryption of filenames, where IVs are
> > +  reused out of necessity.  With a unique key per directory, IV reuse
> > +  is limited to within a single directory.
> 
> Side note: Alex's HEH patchset addresses this.  Is there interest in
> the community for us to push for a merge at this point?
> 
> https://www.spinics.net/lists/linux-crypto/msg22411.html

Well, I think there are higher priorities right now, but if we want to start
that conversation we'd first need to send the updated version of the patchset
which implements the latest version of the algorithm
(https://tools.ietf.org/html/draft-cope-heh-01) and has been rebased onto the
latest kernel.

> > +Note: this KDF meets the primary security requirement, which is to
> > +produce unique derived keys that preserve the entropy of the master
> > +key, assuming that the master key is already a good pseudorandom key.
> > +However, it is nonstandard and has some theoretical problems such as
> > +being reversible, so it is generally considered to be a mistake!  It
> 
> Being reversible isn't theoretical -- it's trivially reversible *if*
> you can exploit kernel memory.  The attack I'm concerned about is that
> an adversary is somehow able to get to an inode key but isn't able to
> get to the master key.  Exploiting kernel memory wasn't in my
> adversarial model at the time, and when I suggested going to something
> like HKDF, Ted convinced me that it would be too painful since he had
> already merged the code.
> 
> Regardless, I'm happy to call it a mistake.
> 
> > +may be replaced with HKDF or another more standard KDF in the future.
> 
> Again, I should point out that your patchset that includes migrating
> to HKDF is still pending merge.  I'd like to see that go in soon.

It is too late for v4.14 since that merge window is already open.  It could
potentially go into v4.15, though as a prerequisite for that I expect that Ted
would like to see more people review/discuss it first.  Note that we do not want
to rush it, since we will be locked into the new on-disk format once merged.

- Eric
--
To unsubscribe from

[PATCH] device: Fix link to device power management documentation

[Geert Uytterhoeven]

Correct location as of commit 2728b2d2e5be4b82 ("PM / core / docs:
Convert sleep states API document to reST").

Signed-off-by: Geert Uytterhoeven 
---
Note that the link was already broken before...
---
 include/linux/device.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/include/linux/device.h b/include/linux/device.h
index beabdbc0842059b3..9db9c528bb5d1608 100644
--- a/include/linux/device.h
+++ b/include/linux/device.h
@@ -838,7 +838,7 @@ struct dev_links_info {
  * @driver_data: Private pointer for driver specific info.
  * @links: Links to suppliers and consumers of this device.
  * @power: For device power management.
- * See Documentation/power/admin-guide/devices.rst for details.
+ * See Documentation/driver-api/pm/devices.rst for details.
  * @pm_domain: Provide callbacks that are executed during system suspend,
  * hibernation, system resume and during runtime PM transitions
  * along with subsystem-level and driver-level callbacks.
-- 
2.7.4

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

Re: [PATCH v2] fscrypt: add a documentation file for filesystem-level encryption

[Joe Richey]

On Thu, Aug 31, 2017 at 5:02 PM, Eric Biggers  wrote:
> From: Eric Biggers 
>
> Perhaps long overdue, add a documentation file for filesystem-level
> encryption, a.k.a. fscrypt or fs/crypto/, to the Documentation
> directory.  The new file is based loosely on the latest version of the
> "EXT4 Encryption Design Document (public version)" Google Doc, but with
> many improvements made, including:
>
> - Reflect the reality that it is not specific to ext4 anymore.
> - More thoroughly document the design and user-visible API/behavior.
> - Replace outdated information, such as the outdated explanation of how
>   encrypted filenames are hashed for indexed directories and how
>   encrypted filenames are presented to userspace without the key.
>   (This was changed just before release.)
>
> For now the focus is on the design and user-visible API/behavior, not on
> how to add encryption support to a filesystem --- since the internal API
> is still pretty messy and any standalone documentation for it would
> become outdated as things get refactored over time.
>
> Signed-off-by: Eric Biggers 
> ---
> Changes since v1:
> - Mention that using existing userspace tools is preferred
> - Don't start an argument about the best way to get random numbers
> - Make it clear that backup/restore of encrypted files without key is
>   not supported yet
> - Mention a reason why the encryption xattr should not be exposed
>   via the xattr system calls
>
>  Documentation/filesystems/fscrypt.rst | 597 
> ++
>  Documentation/filesystems/index.rst   |  11 +
>  MAINTAINERS   |   1 +
>  3 files changed, 609 insertions(+)
>  create mode 100644 Documentation/filesystems/fscrypt.rst
>
> diff --git a/Documentation/filesystems/fscrypt.rst 
> b/Documentation/filesystems/fscrypt.rst
> new file mode 100644
> index ..ec4cad049dde
> --- /dev/null
> +++ b/Documentation/filesystems/fscrypt.rst
> @@ -0,0 +1,597 @@
> +=
> +Filesystem-level encryption (fscrypt)
> +=
> +
> +Introduction
> +
> +
> +fscrypt is a library which filesystems can hook into to support
> +transparent encryption of files and directories.
> +
> +Note: "fscrypt" in this document refers to the kernel-level portion,
> +implemented in ``fs/crypto/``, as opposed to the userspace tool
> +`fscrypt `_.  This document only
> +covers the kernel-level portion.  For command-line examples of how to
> +use encryption, see the documentation for the userspace tool `fscrypt
> +`_.  Also, it is strongly
> +recommended to use the fscrypt userspace tool, or other existing
> +userspace tools such as Android's key management system, over using
> +the kernel's API directly.  Using existing tools reduces the chance of
> +introducing your own security bugs.  (Nevertheless, for completeness
> +this documentation covers the kernel's API anyway.)

I think we should also mention fscryptctl (https://github.com/google/fscryptctl)
here as well. While we should definitly emphasize that fscrypt is the perferred
solution, Richard Weinberger mentioned a need for a smaller simpiler tool when
you just want to apply a static key to a directory. Fscryptctl also had no
shared library dependanceis unlike fscrypt.

- Joe Richey 

> +
> +Unlike dm-crypt, fscrypt operates at the filesystem level rather than
> +at the block device level.  This allows it to encrypt different files
> +with different keys and to have unencrypted files on the same
> +filesystem.  This is useful for multi-user systems where each user's
> +data-at-rest needs to be cryptographically isolated from the others.
> +However, except for filenames, fscrypt does not encrypt filesystem
> +metadata.
> +
> +Unlike eCryptfs, which is a stacked filesystem, fscrypt is integrated
> +directly into supported filesystems --- currently ext4, F2FS, and
> +UBIFS.  This allows encrypted files to be read and written without
> +caching both the decrypted and encrypted pages in the pagecache,
> +thereby halving the memory used and bringing it in line with
> +unencrypted files.  Similarly, half as many dentries and inodes are
> +needed.  eCryptfs also limits filenames to 143 bytes, causing
> +application compatibility issues; fscrypt allows the full 255 bytes
> +(NAME_MAX).  Finally, unlike eCryptfs, the fscrypt API can be used by
> +unprivileged users, with no need to mount anything.
> +
> +fscrypt does not support encrypting files in-place.  Instead, it
> +supports marking an empty directory as encrypted.  Then, after
> +userspace provides the key, all regular files, directories, and
> +symbolic links created in that directory tree are transparently
> +encrypted.
> +
> +Threat model
> +
> +
> +Offline attacks
> +---
> +
> +Provided that userspace chooses a strong encryption key, fscrypt
> +protects the confidentiality of file contents 

Re: [v7 5/5] mm, oom: cgroup v2 mount option to disable cgroup-aware OOM killer

[Roman Gushchin]

On Tue, Sep 05, 2017 at 05:12:51PM +0200, Michal Hocko wrote:
> On Tue 05-09-17 15:30:21, Roman Gushchin wrote:
> > On Tue, Sep 05, 2017 at 03:44:12PM +0200, Michal Hocko wrote:
> [...]
> > > Why is this an opt out rather than opt-in? IMHO the original oom logic
> > > should be preserved by default and specific workloads should opt in for
> > > the cgroup aware logic. Changing the global behavior depending on
> > > whether cgroup v2 interface is in use is more than unexpected and IMHO
> > > wrong approach to take. I think we should instead go with 
> > > oom_strategy=[alloc_task,biggest_task,cgroup]
> > > 
> > > we currently have alloc_task (via sysctl_oom_kill_allocating_task) and
> > > biggest_task which is the default. You are adding cgroup and the more I
> > > think about the more I agree that it doesn't really make sense to try to
> > > fit thew new semantic into the existing one (compare tasks to kill-all
> > > memcgs). Just introduce a new strategy and define a new semantic from
> > > scratch. Memcg priority and kill-all are a natural extension of this new
> > > strategy. This will make the life easier and easier to understand by
> > > users.
> > > 
> > > Does that make sense to you?
> > 
> > Absolutely.
> > 
> > The only thing: I'm not sure that we have to preserve the existing logic
> > as default option. For most users (except few very specific usecases),
> > it should be at least as good, as the existing one.
> 
> But this is really an unexpected change. Users even might not know that
> they are using cgroup v2 and memcg is in use.
> 
> > Making it opt-in means that corresponding code will be executed only
> > by few users, who cares.
> 
> Yeah, which is the way we should introduce new features no?
> 
> > Then we should probably hide corresponding
> > cgroup interface (oom_group and oom_priority knobs) by default,
> > and it feels as unnecessary complication and is overall against
> > cgroup v2 interface design.
> 
> Why. If we care enough, we could simply return EINVAL when those knobs
> are written while the corresponding strategy is not used.

It doesn't look as a nice default interface.

> 
> > > I think we should instead go with
> > > oom_strategy=[alloc_task,biggest_task,cgroup]
> > 
> > It would be a really nice interface; although I've no idea how to implement 
> > it:
> > "alloc_task" is an existing sysctl, which we have to preserve;
> 
> I would argue that we should simply deprecate and later drop the sysctl.
> I _strongly_ suspect anybody is using this. If yes it is not that hard
> to change the kernel command like rather than select the sysctl.

I agree. And if so, why do we need a new interface for an useless feature?

> 
> > while "cgroup" depends on cgroup v2.
> 
> Which is not a big deal either. Simply fall back to default if there are
> no cgroup v2. The implementation would have essentially the same effect
> because there won't be any kill-all cgroups and so we will select the
> largest task.

I'd agree with you, if there are use cases (excluding pure legacy),
when the per-process algorithm is preferable over the cgroup-aware OOM.
I really doubt, and hope, that with oom_priorities the suggested algorithm
should cover almost all reasonable use cases.

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

Re: [v7 2/5] mm, oom: cgroup-aware OOM killer

[Roman Gushchin]

On Tue, Sep 05, 2017 at 04:57:00PM +0200, Michal Hocko wrote:
> On Mon 04-09-17 15:21:05, Roman Gushchin wrote:
> [...]
> > diff --git a/mm/memcontrol.c b/mm/memcontrol.c
> > index a69d23082abf..97813c56163b 100644
> > --- a/mm/memcontrol.c
> > +++ b/mm/memcontrol.c
> > @@ -2649,6 +2649,213 @@ static inline bool memcg_has_children(struct 
> > mem_cgroup *memcg)
> > return ret;
> >  }
> >  
> > +static long memcg_oom_badness(struct mem_cgroup *memcg,
> > + const nodemask_t *nodemask)
> > +{
> > +   long points = 0;
> > +   int nid;
> > +   pg_data_t *pgdat;
> > +
> > +   for_each_node_state(nid, N_MEMORY) {
> > +   if (nodemask && !node_isset(nid, *nodemask))
> > +   continue;
> > +
> > +   points += mem_cgroup_node_nr_lru_pages(memcg, nid,
> > +   LRU_ALL_ANON | BIT(LRU_UNEVICTABLE));
> 
> Why don't you consider file LRUs here? What if there is a lot of page
> cache which is not reclaimed because it is protected by memcg->low.
> Should we hide that from the OOM killer?

I'm not sure here.
I agree with your argument, although memcg->low should not cause OOMs
in the current implementation (which is a separate problem).
Also I can imagine some edge cases with mlocked pagecache belonging
to a process from a different cgroup.

I would suggest to refine this later.

> 
> [...]
> > +static void select_victim_memcg(struct mem_cgroup *root, struct 
> > oom_control *oc)
> > +{
> > +   struct mem_cgroup *iter, *parent;
> > +
> > +   for_each_mem_cgroup_tree(iter, root) {
> > +   if (memcg_has_children(iter)) {
> > +   iter->oom_score = 0;
> > +   continue;
> > +   }
> 
> Do we really need this check? If it is a mere optimization then
> we should probably check for tasks in the memcg rather than
> descendant. More on that below.

The idea is to traverse memcg only once: we're resetting oom_score
for non-leaf cgroups, and for each leaf cgroup calculate the score
and propagate it upwards.

> 
> > +
> > +   iter->oom_score = oom_evaluate_memcg(iter, oc->nodemask);
> > +
> > +   /*
> > +* Ignore empty and non-eligible memory cgroups.
> > +*/
> > +   if (iter->oom_score == 0)
> > +   continue;
> > +
> > +   /*
> > +* If there are inflight OOM victims, we don't need to look
> > +* further for new victims.
> > +*/
> > +   if (iter->oom_score == -1) {
> > +   oc->chosen_memcg = INFLIGHT_VICTIM;
> > +   mem_cgroup_iter_break(root, iter);
> > +   return;
> > +   }
> > +
> > +   for (parent = parent_mem_cgroup(iter); parent && parent != root;
> > +parent = parent_mem_cgroup(parent))
> > +   parent->oom_score += iter->oom_score;
> 
> Hmm. The changelog says "By default, it will look for the biggest leaf
> cgroup, and kill the largest task inside." But you are accumulating
> oom_score up the hierarchy and so parents will have higher score than
> the layer of their children and the larger the sub-hierarchy the more
> biased it will become. Say you have
>   root
>  /\
> /  \
>AD
>   / \
>  B   C
> 
> B (5), C(15) thus A(20) and D(20). Unless I am missing something we are
> going to go down A path and then chose C even though D is the largest
> leaf group, right?

You're right, changelog is not accurate, I'll fix it.
The behavior is correct, IMO.

> 
> > +   }
> > +
> > +   for (;;) {
> > +   struct cgroup_subsys_state *css;
> > +   struct mem_cgroup *memcg = NULL;
> > +   long score = LONG_MIN;
> > +
> > +   css_for_each_child(css, &root->css) {
> > +   struct mem_cgroup *iter = mem_cgroup_from_css(css);
> > +
> > +   /*
> > +* Ignore empty and non-eligible memory cgroups.
> > +*/
> > +   if (iter->oom_score == 0)
> > +   continue;
> > +
> > +   if (iter->oom_score > score) {
> > +   memcg = iter;
> > +   score = iter->oom_score;
> > +   }
> > +   }
> > +
> > +   if (!memcg) {
> > +   if (oc->memcg && root == oc->memcg) {
> > +   oc->chosen_memcg = oc->memcg;
> > +   css_get(&oc->chosen_memcg->css);
> > +   oc->chosen_points = oc->memcg->oom_score;
> > +   }
> > +   break;
> > +   }
> > +
> > +   if (memcg->oom_group || !memcg_has_children(memcg)) {
> > +   oc->chosen_memcg = memcg;
> > +   css_get(&oc->chosen_memcg->css);
> > +   oc->chosen_points = score;
> > +   break;
> > +   }
> > +
> > +   root = memcg;

Re: [PATCH] device: Fix link to device power management documentation

[Rafael J. Wysocki]

On Tue, Sep 5, 2017 at 8:16 PM, Geert Uytterhoeven
 wrote:
> Correct location as of commit 2728b2d2e5be4b82 ("PM / core / docs:
> Convert sleep states API document to reST").
>
> Signed-off-by: Geert Uytterhoeven 
> ---
> Note that the link was already broken before...

Yup, thanks, I'll queue this up.
--
To unsubscribe from this list: send the line "unsubscribe linux-doc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v7 9/9] sparc64: Add support for ADI (Application Data Integrity)

[David Miller]

From: Pavel Machek 
Date: Mon, 4 Sep 2017 18:25:30 +0200

> Will gcc be able to compile code that uses these automatically? That
> does not sound easy to me. Can libc automatically use this in malloc()
> to prevent accessing freed data when buffers are overrun?
> 
> Is this for benefit of JITs?

Anything that can control mappings and the virtual address used to
access memory can use ADI.

malloc() is of course one such case.  It can map memory with ADI
enabled, and return buffer addresses to malloc() callers with the
proper virtual address bits set to satisfy the ADI key checks.

And by induction anything using malloc() for it's memory allocation
gets ADI protection as well.
--
To unsubscribe from this list: send the line "unsubscribe linux-doc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [v7 5/5] mm, oom: cgroup v2 mount option to disable cgroup-aware OOM killer

[Johannes Weiner]

On Tue, Sep 05, 2017 at 03:44:12PM +0200, Michal Hocko wrote:
> Why is this an opt out rather than opt-in? IMHO the original oom logic
> should be preserved by default and specific workloads should opt in for
> the cgroup aware logic. Changing the global behavior depending on
> whether cgroup v2 interface is in use is more than unexpected and IMHO
> wrong approach to take. I think we should instead go with 
> oom_strategy=[alloc_task,biggest_task,cgroup]
> 
> we currently have alloc_task (via sysctl_oom_kill_allocating_task) and
> biggest_task which is the default. You are adding cgroup and the more I
> think about the more I agree that it doesn't really make sense to try to
> fit thew new semantic into the existing one (compare tasks to kill-all
> memcgs). Just introduce a new strategy and define a new semantic from
> scratch. Memcg priority and kill-all are a natural extension of this new
> strategy. This will make the life easier and easier to understand by
> users.

oom_kill_allocating_task is actually a really good example of why
cgroup-awareness *should* be the new default.

Before we had the oom killer victim selection, we simply killed the
faulting/allocating task. While a valid answer to the problem, it's
not very fair or representative of what the user wants or intends.

Then we added code to kill the biggest offender instead, which should
have been the case from the start and was hence made the new default.
The oom_kill_allocating_task was added on the off-chance that there
might be setups who, for historical reasons, rely on the old behavior.
But our default was chosen based on what behavior is fair, expected,
and most reflective of the user's intentions.

The cgroup-awareness in the OOM killer is exactly the same thing. It
should have been the default from the beginning, because the user
configures a group of tasks to be an interdependent, terminal unit of
memory consumption, and it's undesirable for the OOM killer to ignore
this intention and compare members across these boundaries.

We should go the same way here as with kill_alloc_task: the default
should be what's sane and expected by the vast majority of our users,
with a knob (I would prefer a sysctl here, actually) to switch back in
case somebody - unexpectedly - actually relies on the old behavior.

I don't see why couldn't change user-visible behavior here if we don't
expect anyone (quirky setups aside) to rely on it AND we provide a
knob to revert in the field for the exceptions.
--
To unsubscribe from this list: send the line "unsubscribe linux-doc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] hwmon: pmbus: Make reg check and clear faults functions return errors

[Andrew Jeffery]

On Tue, 2017-09-05 at 10:00 -0700, Guenter Roeck wrote:
> On Tue, Sep 05, 2017 at 05:01:32PM +1000, Andrew Jeffery wrote:
> > Some functions exposed by pmbus core conflated errors that occurred when
> > setting the page to access with errors that occurred when accessing
> > registers in a page. In some cases, this caused legitimate errors to be
> > hidden under the guise of the register not being supported.
> > 
> 
> I'll have to look into this. If I recall correctly, the reason for not 
> returning
> errors from the "clear faults" command was that some early chips did not 
> support
> it, or did not support it on all pages. Those chips would now always fail.

Yes, that is a concern.

However, shouldn't the lack of support for CLEAR_FAULTS be a described
property of the chip or page?

Regardless, the issue here is the PAGE command is sometimes failing
(more below). This means we won't have issued a CLEAR_FAULTS against
the page even if the page supports CLEAR_FAULTS. That to me is
something that should be propagated back up the call chain, as faults
that can be cleared will not have been.

> 
> On a higher level, I am not sure if it is a good idea to return an error
> from a function intended to clear faults (and nothing else). That seems
> counterproductive.
> 
> Is this a problem you have actually observed ? 

Unfortunately yes. I was trying to reproduce some issues that we are
seeing in userspace but wasn't having much luck (getting -EIO on
reading a hwmon attribute). I ended up instrumenting our I2C bus
driver, and found that the problem was more prevalent than what the
read failures in userspace were indicating. The paths that were
triggering these unreported conditions all traced through the check
register and clear fault functions, which on analysis were swallowing
the error.

> If so, what is the chip ?

Well, the chip that I'm *communicating* with is the Maxim MAX31785
Intelligent Fan Controller. As to whether that's what is *causing* the
PAGE command to fail is still up in the air. I would not be surprised
if we have other issues in the hardware design.

I haven't sent a follow-up to the series introducing the MAX31785
driver because I've been chasing down communication issues. I felt it
was important to understand whether the device has quirks that need to
be worked around in the driver, or if our hardware design has bigger
problems that should be handled in other ways. I've been in touch with
Maxim who have asserted that some of the problems we're seeing cannot
be caused by their chip.

Andrew

> 
> Thanks,
> Guenter
> 
> > > > Signed-off-by: Andrew Jeffery 
> > ---
> >  Documentation/hwmon/pmbus-core   |  12 ++--
> >  drivers/hwmon/pmbus/pmbus.h  |   6 +-
> >  drivers/hwmon/pmbus/pmbus_core.c | 115 
> > +--
> >  3 files changed, 95 insertions(+), 38 deletions(-)
> > 
> > diff --git a/Documentation/hwmon/pmbus-core b/Documentation/hwmon/pmbus-core
> > index 8ed10e9ddfb5..3e9f41bb756f 100644
> > --- a/Documentation/hwmon/pmbus-core
> > +++ b/Documentation/hwmon/pmbus-core
> > @@ -218,17 +218,17 @@ Specifically, it provides the following information.
> >    This function calls the device specific write_byte function if defined.
> >    Therefore, it must _not_ be called from that function.
> >  
> > -  bool pmbus_check_byte_register(struct i2c_client *client, int page, int 
> > reg);
> > +  int pmbus_check_byte_register(struct i2c_client *client, int page, int 
> > reg);
> >  
> > -  Check if byte register exists. Return true if the register exists, false
> > -  otherwise.
> > +  Check if byte register exists. Returns 1 if the register exists, 0 if it 
> > does
> > +  not, and less than zero on an unexpected error.
> >    This function calls the device specific write_byte function if defined to
> >    obtain the chip status. Therefore, it must _not_ be called from that 
> > function.
> >  
> > -  bool pmbus_check_word_register(struct i2c_client *client, int page, int 
> > reg);
> > +  int pmbus_check_word_register(struct i2c_client *client, int page, int 
> > reg);
> >  
> > -  Check if word register exists. Return true if the register exists, false
> > -  otherwise.
> > +  Check if word register exists. Returns 1 if the register exists, 0 if it 
> > does
> > +  not, and less than zero on an unexpected error.
> >    This function calls the device specific write_byte function if defined to
> >    obtain the chip status. Therefore, it must _not_ be called from that 
> > function.
> >  
> > diff --git a/drivers/hwmon/pmbus/pmbus.h b/drivers/hwmon/pmbus/pmbus.h
> > index bfcb13bae34b..c53032a04a6f 100644
> > --- a/drivers/hwmon/pmbus/pmbus.h
> > +++ b/drivers/hwmon/pmbus/pmbus.h
> > @@ -413,9 +413,9 @@ int pmbus_write_byte_data(struct i2c_client *client, 
> > int page, u8 reg,
> > > >       u8 value);
> >  int pmbus_update_byte_data(struct i2c_client *client, int page, u8 reg,
> > > >        u8 mask, u8 value);
> > -void p