Re: [PATCH v4 RFT 1/2] powerpc/fadump: reduce memory consumption for capture kernel
On Wed, 3 May 2017 12:43:33 +0530
Hari Bathini wrote:
> On Tuesday 02 May 2017 09:26 PM, Michal Suchanek wrote:
> > With fadump (dump capture) kernel booting like a regular kernel, it
> > almost needs the same amount of memory to boot as the production
> > kernel, which is unwarranted for a dump capture kernel. But with no
> > option to disable some of the unnecessary subsystems in fadump
> > kernel, that much memory is wasted on fadump, depriving the
> > production kernel of that memory.
> >
> > Introduce kernel parameter 'fadump_append=' that would take regular
> > kernel parameters to be appended when fadump is active.
> > This 'fadump_append=' parameter can be leveraged to pass parameters
> > like nr_cpus=1, cgroup_disable=memory and numa=off, to disable
> > unwarranted resources/subsystems.
> >
> > Also, ensure the log "Firmware-assisted dump is active" is printed
> > early in the boot process to put the subsequent fadump messages in
> > context.
> >
> > Suggested-by: Michael Ellerman
> > Signed-off-by: Hari Bathini
> > Signed-off-by: Michal Suchanek
> > ---
> > v4:
> > - use space separated arguments instead of comma separated
> > - do not append parameters when fadummp is disabled
> > ---
> > arch/powerpc/kernel/fadump.c | 27 ---
> > 1 file changed, 24 insertions(+), 3 deletions(-)
> >
> > diff --git a/arch/powerpc/kernel/fadump.c
> > b/arch/powerpc/kernel/fadump.c index ebf2e9c..e0c728a 100644
> > --- a/arch/powerpc/kernel/fadump.c
> > +++ b/arch/powerpc/kernel/fadump.c
> > @@ -79,8 +79,10 @@ int __init early_init_dt_scan_fw_dump(unsigned
> > long node,
> > * dump data waiting for us.
> > */
> > fdm_active = of_get_flat_dt_prop(node, "ibm,kernel-dump",
> > NULL);
> > - if (fdm_active)
> > + if (fdm_active) {
> > + pr_info("Firmware-assisted dump is active.\n");
> > fw_dump.dump_active = 1;
> > + }
> >
> > /* Get the sizes required to store dump data for the
> > firmware provided
> > * dump sections.
> > @@ -263,8 +265,12 @@ int __init fadump_reserve_mem(void)
> > {
> > unsigned long base, size, memory_boundary;
> >
> > - if (!fw_dump.fadump_enabled)
> > + if (!fw_dump.fadump_enabled) {
> > + if (fw_dump.dump_active)
> > + pr_warn("Firmware-assisted dump was active
> > but kernel"
> > + " booted with fadump disabled!\n");
> > return 0;
> > + }
> >
> > if (!fw_dump.fadump_supported) {
> > printk(KERN_INFO "Firmware-assisted dump is not
> > supported on" @@ -304,7 +310,6 @@ int __init
> > fadump_reserve_mem(void) memory_boundary = memblock_end_of_DRAM();
> >
> > if (fw_dump.dump_active) {
> > - printk(KERN_INFO "Firmware-assisted dump is
> > active.\n"); /*
> > * If last boot has crashed then reserve all the
> > memory
> > * above boot_memory_size so that we don't touch
> > it until @@ -363,6 +368,22 @@ unsigned long __init
> > arch_reserved_kernel_pages(void) return memblock_reserved_size() /
> > PAGE_SIZE; }
> >
> > +/* Look for fadump_append= cmdline option. */
> > +static int __init early_fadump_append_param(char *p)
> > +{
> > + if (!p)
> > + return 1;
> > +
> > + if (fw_dump.fadump_enabled && fw_dump.dump_active) {
> > + pr_info("enforcing additional parameters (%s) passed
> > through "
> > + "'fadump_append=' parameter\n", p);
> > + parse_early_options(p);
>
> While testing this on a system with yaboot bootloader, it seems that
> parsing a parameter with syntax like fadump_append="nr_cpus=1
> numa=off" is not resulting in the intended way..
Since yaboot arguments are quoted you will probably want something like
append = "quiet splash somearg1 somearg2 fadump_append=\"nr_cpus=1
numa=off\""
in yaboot.conf
Looking at the yaboot parser it seems to handle quoting of quotes so
unlike your parser that does not handle quoting of commas it allows
passing arbitrary arguments to both the normal kernel and the fadump
kernel.
Thanks
Michal
--
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 v4 RFT 1/2] powerpc/fadump: reduce memory consumption for capture kernel
On Wednesday 10 May 2017 08:28 PM, Michal Suchánek wrote:
On Wed, 3 May 2017 12:43:33 +0530
Hari Bathini wrote:
On Tuesday 02 May 2017 09:26 PM, Michal Suchanek wrote:
With fadump (dump capture) kernel booting like a regular kernel, it
almost needs the same amount of memory to boot as the production
kernel, which is unwarranted for a dump capture kernel. But with no
option to disable some of the unnecessary subsystems in fadump
kernel, that much memory is wasted on fadump, depriving the
production kernel of that memory.
Introduce kernel parameter 'fadump_append=' that would take regular
kernel parameters to be appended when fadump is active.
This 'fadump_append=' parameter can be leveraged to pass parameters
like nr_cpus=1, cgroup_disable=memory and numa=off, to disable
unwarranted resources/subsystems.
Also, ensure the log "Firmware-assisted dump is active" is printed
early in the boot process to put the subsequent fadump messages in
context.
Suggested-by: Michael Ellerman
Signed-off-by: Hari Bathini
Signed-off-by: Michal Suchanek
---
v4:
- use space separated arguments instead of comma separated
- do not append parameters when fadummp is disabled
---
arch/powerpc/kernel/fadump.c | 27 ---
1 file changed, 24 insertions(+), 3 deletions(-)
diff --git a/arch/powerpc/kernel/fadump.c
b/arch/powerpc/kernel/fadump.c index ebf2e9c..e0c728a 100644
--- a/arch/powerpc/kernel/fadump.c
+++ b/arch/powerpc/kernel/fadump.c
@@ -79,8 +79,10 @@ int __init early_init_dt_scan_fw_dump(unsigned
long node,
* dump data waiting for us.
*/
fdm_active = of_get_flat_dt_prop(node, "ibm,kernel-dump",
NULL);
- if (fdm_active)
+ if (fdm_active) {
+ pr_info("Firmware-assisted dump is active.\n");
fw_dump.dump_active = 1;
+ }
/* Get the sizes required to store dump data for the
firmware provided
* dump sections.
@@ -263,8 +265,12 @@ int __init fadump_reserve_mem(void)
{
unsigned long base, size, memory_boundary;
- if (!fw_dump.fadump_enabled)
+ if (!fw_dump.fadump_enabled) {
+ if (fw_dump.dump_active)
+ pr_warn("Firmware-assisted dump was active
but kernel"
+ " booted with fadump disabled!\n");
return 0;
+ }
if (!fw_dump.fadump_supported) {
printk(KERN_INFO "Firmware-assisted dump is not
supported on" @@ -304,7 +310,6 @@ int __init
fadump_reserve_mem(void) memory_boundary = memblock_end_of_DRAM();
if (fw_dump.dump_active) {
- printk(KERN_INFO "Firmware-assisted dump is
active.\n"); /*
* If last boot has crashed then reserve all the
memory
* above boot_memory_size so that we don't touch
it until @@ -363,6 +368,22 @@ unsigned long __init
arch_reserved_kernel_pages(void) return memblock_reserved_size() /
PAGE_SIZE; }
+/* Look for fadump_append= cmdline option. */
+static int __init early_fadump_append_param(char *p)
+{
+ if (!p)
+ return 1;
+
+ if (fw_dump.fadump_enabled && fw_dump.dump_active) {
+ pr_info("enforcing additional parameters (%s) passed
through "
+ "'fadump_append=' parameter\n", p);
+ parse_early_options(p);
While testing this on a system with yaboot bootloader, it seems that
parsing a parameter with syntax like fadump_append="nr_cpus=1
numa=off" is not resulting in the intended way..
Since yaboot arguments are quoted you will probably want something like
append = "quiet splash somearg1 somearg2 fadump_append=\"nr_cpus=1
numa=off\""
I did try this out. But it wasn't helping either..
in yaboot.conf
Looking at the yaboot parser it seems to handle quoting of quotes so
unlike your parser that does not handle quoting of commas it allows
passing arbitrary arguments to both the normal kernel and the fadump
kernel.
Will improve on v5 to ensure quotes are taken care of..
Thanks
Hari
--
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: [RFC 09/10] ima: move to generic async completion
On Sat, 2017-05-06 at 15:59 +0300, Gilad Ben-Yossef wrote:
> 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 802d5d2..0e4db1fe 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);
> @@ -527,7 +505,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);
> @@ -536,12 +514,12 @@ static int calc_buffer_ahash_atfm(const void *buf,
> loff_t len,
> if (!req)
> return -ENOMEM;
>
> - init_completion(&res.completion);
> + crypt
[PATCH v3 2/4] hwmon: (adt7475) fan stall prevention
By default adt7475 will stop the fans (pwm duty cycle 0%) when the
temperature drops past Tmin - hysteresis. Some systems want to keep the
fans moving even when the temperature drops so add new sysfs attributes
that configure the enhanced acoustics min 1-3 which allows the fans to
run at the minimum configure pwm duty cycle.
Signed-off-by: Chris Packham
---
Changes in v2:
- use pwmN_stall_dis as the attribute name. I think this describes the purpose
pretty well. I went with a new attribute instead of overloading
pwmN_auto_point1_pwm so this doesn't affect existing users.
Changes in v3:
- Fix grammar.
- change enh_acou to enh_acoustics
Documentation/hwmon/adt7475 | 5 +
drivers/hwmon/adt7475.c | 50 +
2 files changed, 55 insertions(+)
diff --git a/Documentation/hwmon/adt7475 b/Documentation/hwmon/adt7475
index 0502f2b464e1..3990bae60e78 100644
--- a/Documentation/hwmon/adt7475
+++ b/Documentation/hwmon/adt7475
@@ -109,6 +109,11 @@ fan speed) is applied. PWM values range from 0 (off) to
255 (full speed).
Fan speed may be set to maximum when the temperature sensor associated with
the PWM control exceeds temp#_max.
+At Tmin - hysteresis the PWM output can either be off (0% duty cycle) or at the
+minimum (i.e. auto_point1_pwm). This behaviour can be configured using the
+pwm[1-*]_stall_dis sysfs attribute. A value of 0 means the fans will shut off.
+A value of 1 means the fans will run at auto_point1_pwm.
+
Notes
-
diff --git a/drivers/hwmon/adt7475.c b/drivers/hwmon/adt7475.c
index ec0c43fbcdce..4d6c625fec70 100644
--- a/drivers/hwmon/adt7475.c
+++ b/drivers/hwmon/adt7475.c
@@ -79,6 +79,9 @@
#define REG_TEMP_TRANGE_BASE 0x5F
+#define REG_ENHANCE_ACOUSTICS1 0x62
+#define REG_ENHANCE_ACOUSTICS2 0x63
+
#define REG_PWM_MIN_BASE 0x64
#define REG_TEMP_TMIN_BASE 0x67
@@ -209,6 +212,7 @@ struct adt7475_data {
u8 range[3];
u8 pwmctl[3];
u8 pwmchan[3];
+ u8 enh_acoustics[2];
u8 vid;
u8 vrm;
@@ -700,6 +704,43 @@ static ssize_t set_pwm(struct device *dev, struct
device_attribute *attr,
data->pwm[sattr->nr][sattr->index] = clamp_val(val, 0, 0xFF);
i2c_smbus_write_byte_data(client, reg,
data->pwm[sattr->nr][sattr->index]);
+ mutex_unlock(&data->lock);
+
+ return count;
+}
+
+
+static ssize_t show_stall_dis(struct device *dev, struct device_attribute
*attr,
+ char *buf)
+{
+ struct sensor_device_attribute_2 *sattr = to_sensor_dev_attr_2(attr);
+ struct i2c_client *client = to_i2c_client(dev);
+ struct adt7475_data *data = i2c_get_clientdata(client);
+ u8 mask = BIT(5 + sattr->index);
+
+ return sprintf(buf, "%d\n", !!(data->enh_acoustics[0] & mask));
+}
+
+static ssize_t set_stall_dis(struct device *dev, struct device_attribute *attr,
+const char *buf, size_t count)
+{
+ struct sensor_device_attribute_2 *sattr = to_sensor_dev_attr_2(attr);
+ struct i2c_client *client = to_i2c_client(dev);
+ struct adt7475_data *data = i2c_get_clientdata(client);
+ long val;
+ u8 mask = BIT(5 + sattr->index);
+
+ if (kstrtol(buf, 10, &val))
+ return -EINVAL;
+
+ mutex_lock(&data->lock);
+
+ data->enh_acoustics[0] &= ~mask;
+ if (val)
+ data->enh_acoustics[0] |= mask;
+
+ i2c_smbus_write_byte_data(client, REG_ENHANCE_ACOUSTICS1,
+ data->enh_acoustics[0]);
mutex_unlock(&data->lock);
@@ -1028,6 +1069,8 @@ static SENSOR_DEVICE_ATTR_2(pwm1_auto_point1_pwm, S_IRUGO
| S_IWUSR, show_pwm,
set_pwm, MIN, 0);
static SENSOR_DEVICE_ATTR_2(pwm1_auto_point2_pwm, S_IRUGO | S_IWUSR, show_pwm,
set_pwm, MAX, 0);
+static SENSOR_DEVICE_ATTR_2(pwm1_stall_dis, S_IRUGO | S_IWUSR, show_stall_dis,
+ set_stall_dis, 0, 0);
static SENSOR_DEVICE_ATTR_2(pwm2, S_IRUGO | S_IWUSR, show_pwm, set_pwm, INPUT,
1);
static SENSOR_DEVICE_ATTR_2(pwm2_freq, S_IRUGO | S_IWUSR, show_pwmfreq,
@@ -1040,6 +1083,8 @@ static SENSOR_DEVICE_ATTR_2(pwm2_auto_point1_pwm, S_IRUGO
| S_IWUSR, show_pwm,
set_pwm, MIN, 1);
static SENSOR_DEVICE_ATTR_2(pwm2_auto_point2_pwm, S_IRUGO | S_IWUSR, show_pwm,
set_pwm, MAX, 1);
+static SENSOR_DEVICE_ATTR_2(pwm2_stall_dis, S_IRUGO | S_IWUSR, show_stall_dis,
+ set_stall_dis, 0, 1);
static SENSOR_DEVICE_ATTR_2(pwm3, S_IRUGO | S_IWUSR, show_pwm, set_pwm, INPUT,
2);
static SENSOR_DEVICE_ATTR_2(pwm3_freq, S_IRUGO | S_IWUSR, show_pwmfreq,
@@ -1052,6 +1097,8 @@ static SENSOR_DEVICE_ATTR_2(pwm3_auto_point1_pwm, S_IRUGO
| S_IWUSR, show_pwm,
set_pwm, MIN, 2);
static SENSOR_DEVICE_ATTR_2(pwm3_auto_poin
[PATCH v3 3/4] hwmon: (adt7475) temperature smoothing
When enabled temperature smoothing allows ramping the fan speed over a
configurable period of time instead of jumping to the new speed
instantaneously.
Signed-off-by: Chris Packham
---
Changes in v2:
- use a single tempN_smoothing attribute
Changes in v3:
- change enh_acou to enh_acoustics
- simplify show_temp_st()
Documentation/hwmon/adt7475 | 4 ++
drivers/hwmon/adt7475.c | 93 +
2 files changed, 97 insertions(+)
diff --git a/Documentation/hwmon/adt7475 b/Documentation/hwmon/adt7475
index 3990bae60e78..e82b24ec4b07 100644
--- a/Documentation/hwmon/adt7475
+++ b/Documentation/hwmon/adt7475
@@ -114,6 +114,10 @@ minimum (i.e. auto_point1_pwm). This behaviour can be
configured using the
pwm[1-*]_stall_dis sysfs attribute. A value of 0 means the fans will shut off.
A value of 1 means the fans will run at auto_point1_pwm.
+The responsiveness of the ADT747x to temperature changes can be configured.
+This allows smoothing of the fan speed transition. To set the transition time
+set the value in ms in the temp[1-*]_smoothing sysfs attribute.
+
Notes
-
diff --git a/drivers/hwmon/adt7475.c b/drivers/hwmon/adt7475.c
index 4d6c625fec70..f7322330789c 100644
--- a/drivers/hwmon/adt7475.c
+++ b/drivers/hwmon/adt7475.c
@@ -526,6 +526,90 @@ static ssize_t set_temp(struct device *dev, struct
device_attribute *attr,
return count;
}
+/* Assuming CONFIG6[SLOW] is 0 */
+static const int ad7475_st_map[] = {
+ 37500, 18800, 12500, 7500, 4700, 3100, 1600, 800,
+};
+
+static ssize_t show_temp_st(struct device *dev, struct device_attribute *attr,
+ char *buf)
+{
+ struct sensor_device_attribute_2 *sattr = to_sensor_dev_attr_2(attr);
+ struct i2c_client *client = to_i2c_client(dev);
+ struct adt7475_data *data = i2c_get_clientdata(client);
+ long val;
+
+ switch (sattr->index) {
+ case 0:
+ val = data->enh_acoustics[0] & 0xf;
+ break;
+ case 1:
+ val = (data->enh_acoustics[1] >> 4) & 0xf;
+ break;
+ case 2:
+ val = data->enh_acoustics[1] & 0xf;
+ break;
+ default:
+ return -EINVAL;
+ }
+
+ if (val & 0x8)
+ return sprintf(buf, "%d\n", ad7475_st_map[val & 0x7]);
+ else
+ return sprintf(buf, "0\n");
+}
+
+static ssize_t set_temp_st(struct device *dev, struct device_attribute *attr,
+const char *buf, size_t count)
+{
+ struct sensor_device_attribute_2 *sattr = to_sensor_dev_attr_2(attr);
+ struct i2c_client *client = to_i2c_client(dev);
+ struct adt7475_data *data = i2c_get_clientdata(client);
+ unsigned char reg;
+ int shift, idx;
+ ulong val;
+
+ if (kstrtoul(buf, 10, &val))
+ return -EINVAL;
+
+ switch (sattr->index) {
+ case 0:
+ reg = REG_ENHANCE_ACOUSTICS1;
+ shift = 0;
+ idx = 0;
+ break;
+ case 1:
+ reg = REG_ENHANCE_ACOUSTICS2;
+ shift = 4;
+ idx = 1;
+ break;
+ case 2:
+ reg = REG_ENHANCE_ACOUSTICS2;
+ shift = 0;
+ idx = 1;
+ break;
+ default:
+ return -EINVAL;
+ }
+
+ if (val > 0) {
+ val = find_closest_descending(val, ad7475_st_map,
+ ARRAY_SIZE(ad7475_st_map));
+ val |= 0x8;
+ }
+
+ mutex_lock(&data->lock);
+
+ data->enh_acoustics[idx] &= ~(0xf << shift);
+ data->enh_acoustics[idx] |= (val << shift);
+
+ i2c_smbus_write_byte_data(client, reg, data->enh_acoustics[idx]);
+
+ mutex_unlock(&data->lock);
+
+ return count;
+}
+
/*
* Table of autorange values - the user will write the value in millidegrees,
* and we'll convert it
@@ -1008,6 +1092,8 @@ static SENSOR_DEVICE_ATTR_2(temp1_crit, S_IRUGO |
S_IWUSR, show_temp, set_temp,
THERM, 0);
static SENSOR_DEVICE_ATTR_2(temp1_crit_hyst, S_IRUGO | S_IWUSR, show_temp,
set_temp, HYSTERSIS, 0);
+static SENSOR_DEVICE_ATTR_2(temp1_smoothing, S_IRUGO | S_IWUSR, show_temp_st,
+ set_temp_st, 0, 0);
static SENSOR_DEVICE_ATTR_2(temp2_input, S_IRUGO, show_temp, NULL, INPUT, 1);
static SENSOR_DEVICE_ATTR_2(temp2_alarm, S_IRUGO, show_temp, NULL, ALARM, 1);
static SENSOR_DEVICE_ATTR_2(temp2_max, S_IRUGO | S_IWUSR, show_temp, set_temp,
@@ -1024,6 +1110,8 @@ static SENSOR_DEVICE_ATTR_2(temp2_crit, S_IRUGO |
S_IWUSR, show_temp, set_temp,
THERM, 1);
static SENSOR_DEVICE_ATTR_2(temp2_crit_hyst, S_IRUGO | S_IWUSR, show_temp,
set_temp, HYSTERSIS, 1);
+static SENSOR_DEVICE_ATTR_2(temp2_smoothing, S_IRUGO | S_IWUSR, show_temp_st,
+
Re: [RFC 01/10] crypto: factor async completion for general use
Hi Gilad,
On Sat, May 06, 2017 at 03:59:50PM +0300, Gilad Ben-Yossef wrote:
> 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 re-factors one of the in crypto API implementation in
> preparation for using it across the board instead of hand
> rolled versions.
Thanks for doing this! It annoyed me too that there wasn't a helper function
for this. Just a few comments below:
> diff --git a/crypto/af_alg.c b/crypto/af_alg.c
> index 3556d8e..bf4acaf 100644
> --- a/crypto/af_alg.c
> +++ b/crypto/af_alg.c
> @@ -480,33 +480,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);
> -
I think it would be cleaner to switch af_alg and algif_* over to the new
interface in its own patch, rather than in the same patch that introduces the
new interface.
> @@ -88,8 +88,8 @@ 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;
> }
In general can you try to keep the argument lists indented sanely? (This
applies throughout the patch series.) e.g. here I'd have expected:
err = crypto_wait_req(crypto_ahash_init(&ctx->req),
&ctx->wait);
>
> diff --git a/crypto/api.c b/crypto/api.c
> index 941cd4c..1c6e9cd 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,32 @@ int crypto_has_alg(const char *name, u32 type, u32 mask)
> }
> EXPORT_SYMBOL_GPL(crypto_has_alg);
>
> +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;
> +}
> +EXPORT_SYMBOL_GPL(crypto_wait_req);
crypto_wait_req() maybe should be inlined, since it doesn't do much (note that
reinit_completion is actually just a variable assignment), and the common case
is that 'err' will be 0, so there will be nothing to wait for.
Also drop the unnecessary semicolon at the end of the switch block.
With regards to the wait being uninterruptible, I agree that this should be the
default behavior, because I think users waiting for specific crypto requests are
generally not prepared to handle the wait actually being interrupted. After
interruption the crypto operation will still proceed in the background, and it
will use buffers which the caller has in many cases already freed. However, I'd
suggest taking a close look at anything that was actually doing an interruptible
wait before, to see whether it was a bug or intentional (or "doesn't matter").
And yes there could always be a crypto_wait_req_interruptible() introduced if
some users need it.
>
> #define CRYPTO_MINALIGN_ATTR __attribute__ ((__aligned__(CRYPTO_MINALIGN)))
>
> +/*
> + * 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 }
> +
Move this definition down below, so it's next to crypto_wait?
>
> /*
> * Algorithm registration interface.
> */
> @@ -1604,5 +1631,6 @@ static inline int crypto_comp_decompress(struct
> crypto_comp *tfm,
> src, slen, dst, dlen);
> }
>
> +
Don't add unrelated blank lines.
- Eric
--
To unsubscribe from this list: send the line "unsubscribe linux-doc" in
the body of a message
Re: [RFC 07/10] fscrypt: move to generic async completion
Hi Gilad, On Sat, May 06, 2017 at 03:59:56PM +0300, Gilad Ben-Yossef wrote: > 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, > @@ -150,7 +135,7 @@ int fscrypt_do_page_crypto(const struct inode *inode, > fscrypt_direction_t rw, > u8 padding[FS_XTS_TWEAK_SIZE - sizeof(__le64)]; > } xts_tweak; > struct skcipher_request *req = NULL; > - DECLARE_FS_COMPLETION_RESULT(ecr); > + DECLARE_CRYPTO_WAIT(ecr); > struct scatterlist dst, src; > struct fscrypt_info *ci = inode->i_crypt_info; > struct crypto_skcipher *tfm = ci->ci_ctfm; > @@ -168,7 +153,7 @@ int fscrypt_do_page_crypto(const struct inode *inode, > fscrypt_direction_t rw, [...] This patch looks good --- thanks for doing this! I suggest also renaming 'ecr' to 'wait' in the places being updated to use crypto_wait, since the name is obsolete (and was already; it originally stood for "ext4 completion result"). - Eric -- 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
