[PATCH] This is a patch to the capi.c file that fixes one instance of the following error: ERROR: do not use assignment in if condition
Staging: i4l: act200: Signed-off-by: Naeil Zoueidi --- drivers/staging/i4l/act2000/capi.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/drivers/staging/i4l/act2000/capi.c b/drivers/staging/i4l/act2000/capi.c index 62f5629..b494917 100644 --- a/drivers/staging/i4l/act2000/capi.c +++ b/drivers/staging/i4l/act2000/capi.c @@ -564,7 +564,8 @@ actcapi_data_b3_ind(act2000_card *card, struct sk_buff *skb) { blocknr = msg->msg.data_b3_ind.blocknr; skb_pull(skb, 19); card->interface.rcvcallb_skb(card->myid, chan, skb); - if (!(skb = alloc_skb(11, GFP_ATOMIC))) { + skb = alloc_skb(11, GFP_ATOMIC; + if (!skb) { printk(KERN_WARNING "actcapi: alloc_skb failed\n"); return 1; } -- 2.10.1 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] This is a patch to the capi.c file that fixes one instance of the following error: ERROR: do not use assignment in if condition
Hi Naeil, [auto build test ERROR on staging/staging-testing] [also build test ERROR on v4.9-rc4 next-20161028] [if your patch is applied to the wrong git tree, please drop us a note to help improve the system] url: https://github.com/0day-ci/linux/commits/Naeil-ZOUEIDI/This-is-a-patch-to-the-capi-c-file-that-fixes-one-instance-of-the-following-error-ERROR-do-not-use-assignment-in-if-condition/20161106-165820 config: i386-randconfig-x077-201645 (attached as .config) compiler: gcc-6 (Debian 6.2.0-3) 6.2.0 20160901 reproduce: # save the attached .config to linux build tree make ARCH=i386 All error/warnings (new ones prefixed by >>): drivers/staging/i4l/act2000/capi.c: In function 'actcapi_data_b3_ind': >> drivers/staging/i4l/act2000/capi.c:571:32: error: expected ')' before ';' >> token skb = alloc_skb(11, GFP_ATOMIC; ^ >> drivers/staging/i4l/act2000/capi.c:586:1: error: expected ';' before '}' >> token } ^ >> drivers/staging/i4l/act2000/capi.c:586:1: warning: control reaches end of >> non-void function [-Wreturn-type] } ^ vim +571 drivers/staging/i4l/act2000/capi.c 565 return 0; 566 if (card->bch[chan].plci != plci) 567 return 0; 568 blocknr = msg->msg.data_b3_ind.blocknr; 569 skb_pull(skb, 19); 570 card->interface.rcvcallb_skb(card->myid, chan, skb); > 571 skb = alloc_skb(11, GFP_ATOMIC; 572 if (!skb) { 573 printk(KERN_WARNING "actcapi: alloc_skb failed\n"); 574 return 1; 575 } 576 msg = (actcapi_msg *)skb_put(skb, 11); 577 msg->hdr.len = 11; 578 msg->hdr.applicationID = 1; 579 msg->hdr.cmd.cmd = 0x86; 580 msg->hdr.cmd.subcmd = 0x03; 581 msg->hdr.msgnum = actcapi_nextsmsg(card); 582 msg->msg.data_b3_resp.ncci = ncci; 583 msg->msg.data_b3_resp.blocknr = blocknr; 584 ACTCAPI_QUEUE_TX; 585 return 1; > 586 } 587 588 /* 589 * Walk over ackq, unlink DATA_B3_REQ from it, if --- 0-DAY kernel test infrastructureOpen Source Technology Center https://lists.01.org/pipermail/kbuild-all Intel Corporation .config.gz Description: application/gzip ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH 2/2] Adding a missing parenthesis to an assignment and fixing my first patch This patch is a fix to the error ERROR: do not use assignment in if condition found by the checkpatch tool
Staging: i4l: act2000: Signed-off-by: Naeil ZOUEIDI --- drivers/staging/i4l/act2000/capi.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/staging/i4l/act2000/capi.c b/drivers/staging/i4l/act2000/capi.c index b494917..62dee39 100644 --- a/drivers/staging/i4l/act2000/capi.c +++ b/drivers/staging/i4l/act2000/capi.c @@ -564,7 +564,7 @@ actcapi_data_b3_ind(act2000_card *card, struct sk_buff *skb) { blocknr = msg->msg.data_b3_ind.blocknr; skb_pull(skb, 19); card->interface.rcvcallb_skb(card->myid, chan, skb); - skb = alloc_skb(11, GFP_ATOMIC; + skb = alloc_skb(11, GFP_ATOMIC); if (!skb) { printk(KERN_WARNING "actcapi: alloc_skb failed\n"); return 1; -- 2.10.1 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 1/9] staging: iio: tsl2583: i2c_smbus_write_byte() / i2c_smbus_read_byte() migration
On 03/11/16 12:56, Brian Masney wrote: > There were several places where the driver would first call > i2c_smbus_write_byte() to select the register on the device, and then > call i2c_smbus_read_byte() to get the contents of that register. The > code would look roughly like: > > /* Select register */ > i2c_smbus_write_byte(client, REGISTER); > > /* Read the the last register that was written to */ > int data = i2c_smbus_read_byte(client); > > Rewrite this to use i2c_smbus_read_byte_data() to combine the two > calls into one: > > int data = i2c_smbus_read_byte_data(chip->client, REGISTER); > > Verified that the driver still functions correctly using a TSL2581 > hooked up to a Raspberry Pi 2. > > This fixes the following warnings that were found by the > kbuild test robot that were introduced by commit 8ba355cce3c6 ("staging: > iio: tsl2583: check for error code from i2c_smbus_read_byte()"). > > drivers/staging/iio/light/tsl2583.c:365:5-12: WARNING: Unsigned > expression compared with zero: reg_val < 0 > > drivers/staging/iio/light/tsl2583.c:388:5-12: WARNING: Unsigned > expression compared with zero: reg_val < 0 > > This also removes the need for the taos_i2c_read() function since all > callers were only calling the function with a length of 1. > > Signed-off-by: Brian Masney > Cc: Julia Lawall One comment in line. Good to get rid of the warning that the autobuilders have been pinging at me ever time I do a push! Jonathan > --- > drivers/staging/iio/light/tsl2583.c | 85 > +++-- > 1 file changed, 16 insertions(+), 69 deletions(-) > > diff --git a/drivers/staging/iio/light/tsl2583.c > b/drivers/staging/iio/light/tsl2583.c > index 49b19f5..a3a9095 100644 > --- a/drivers/staging/iio/light/tsl2583.c > +++ b/drivers/staging/iio/light/tsl2583.c > @@ -155,39 +155,6 @@ static void taos_defaults(struct tsl2583_chip *chip) > } > > /* > - * Read a number of bytes starting at register (reg) location. > - * Return 0, or i2c_smbus_write_byte ERROR code. > - */ > -static int > -taos_i2c_read(struct i2c_client *client, u8 reg, u8 *val, unsigned int len) > -{ > - int i, ret; > - > - for (i = 0; i < len; i++) { > - /* select register to write */ > - ret = i2c_smbus_write_byte(client, (TSL258X_CMD_REG | reg)); > - if (ret < 0) { > - dev_err(&client->dev, > - "taos_i2c_read failed to write register %x\n", > - reg); > - return ret; > - } > - /* read the data */ > - ret = i2c_smbus_read_byte(client); > - if (ret < 0) { > - dev_err(&client->dev, > - "%s failed to read byte after writing to > register %x\n", > - __func__, reg); > - return ret; > - } > - *val = ret; > - val++; > - reg++; > - } > - return 0; > -} > - > -/* > * Reads and calculates current lux value. > * The raw ch0 and ch1 values of the ambient light sensed in the last > * integration cycle are read from the device. > @@ -220,13 +187,14 @@ static int taos_get_lux(struct iio_dev *indio_dev) > goto done; > } > > - ret = taos_i2c_read(chip->client, (TSL258X_CMD_REG), &buf[0], 1); > + ret = i2c_smbus_read_byte_data(chip->client, TSL258X_CMD_REG); > if (ret < 0) { > dev_err(&chip->client->dev, "taos_get_lux failed to read > CMD_REG\n"); > goto done; > } > + > /* is data new & valid */ > - if (!(buf[0] & TSL258X_STA_ADC_INTR)) { > + if (!(ret & TSL258X_STA_ADC_INTR)) { > dev_err(&chip->client->dev, "taos_get_lux data not valid\n"); > ret = chip->als_cur_info.lux; /* return LAST VALUE */ > goto done; > @@ -235,13 +203,14 @@ static int taos_get_lux(struct iio_dev *indio_dev) > for (i = 0; i < 4; i++) { > int reg = TSL258X_CMD_REG | (TSL258X_ALS_CHAN0LO + i); > > - ret = taos_i2c_read(chip->client, reg, &buf[i], 1); > + ret = i2c_smbus_read_byte_data(chip->client, reg); > if (ret < 0) { > dev_err(&chip->client->dev, > "taos_get_lux failed to read register %x\n", > reg); > goto done; > } > + buf[i] = ret; > } > > /* > @@ -343,52 +312,36 @@ static int taos_get_lux(struct iio_dev *indio_dev) > static int taos_als_calibrate(struct iio_dev *indio_dev) > { > struct tsl2583_chip *chip = iio_priv(indio_dev); > - u8 reg_val; > unsigned int gain_trim_val; > int ret; > int lux_val; > > - ret = i2c_smbus_write_byte(chip->client, > -(TSL258X_CMD_REG | TSL258X_CNTRL)); > + ret = i2c_smbus_read_byte_d
Re: [PATCH 1/9] staging: iio: tsl2583: i2c_smbus_write_byte() / i2c_smbus_read_byte() migration
On 03/11/16 12:56, Brian Masney wrote: > There were several places where the driver would first call > i2c_smbus_write_byte() to select the register on the device, and then > call i2c_smbus_read_byte() to get the contents of that register. The > code would look roughly like: > > /* Select register */ > i2c_smbus_write_byte(client, REGISTER); > > /* Read the the last register that was written to */ > int data = i2c_smbus_read_byte(client); > > Rewrite this to use i2c_smbus_read_byte_data() to combine the two > calls into one: > > int data = i2c_smbus_read_byte_data(chip->client, REGISTER); > > Verified that the driver still functions correctly using a TSL2581 > hooked up to a Raspberry Pi 2. > > This fixes the following warnings that were found by the > kbuild test robot that were introduced by commit 8ba355cce3c6 ("staging: > iio: tsl2583: check for error code from i2c_smbus_read_byte()"). > > drivers/staging/iio/light/tsl2583.c:365:5-12: WARNING: Unsigned > expression compared with zero: reg_val < 0 > > drivers/staging/iio/light/tsl2583.c:388:5-12: WARNING: Unsigned > expression compared with zero: reg_val < 0 > > This also removes the need for the taos_i2c_read() function since all > callers were only calling the function with a length of 1. > > Signed-off-by: Brian Masney > Cc: Julia Lawall Forgot to say - applied to the togreg branch of iio.git and pushed out as testing to see what we've missed! Jonathan > --- > drivers/staging/iio/light/tsl2583.c | 85 > +++-- > 1 file changed, 16 insertions(+), 69 deletions(-) > > diff --git a/drivers/staging/iio/light/tsl2583.c > b/drivers/staging/iio/light/tsl2583.c > index 49b19f5..a3a9095 100644 > --- a/drivers/staging/iio/light/tsl2583.c > +++ b/drivers/staging/iio/light/tsl2583.c > @@ -155,39 +155,6 @@ static void taos_defaults(struct tsl2583_chip *chip) > } > > /* > - * Read a number of bytes starting at register (reg) location. > - * Return 0, or i2c_smbus_write_byte ERROR code. > - */ > -static int > -taos_i2c_read(struct i2c_client *client, u8 reg, u8 *val, unsigned int len) > -{ > - int i, ret; > - > - for (i = 0; i < len; i++) { > - /* select register to write */ > - ret = i2c_smbus_write_byte(client, (TSL258X_CMD_REG | reg)); > - if (ret < 0) { > - dev_err(&client->dev, > - "taos_i2c_read failed to write register %x\n", > - reg); > - return ret; > - } > - /* read the data */ > - ret = i2c_smbus_read_byte(client); > - if (ret < 0) { > - dev_err(&client->dev, > - "%s failed to read byte after writing to > register %x\n", > - __func__, reg); > - return ret; > - } > - *val = ret; > - val++; > - reg++; > - } > - return 0; > -} > - > -/* > * Reads and calculates current lux value. > * The raw ch0 and ch1 values of the ambient light sensed in the last > * integration cycle are read from the device. > @@ -220,13 +187,14 @@ static int taos_get_lux(struct iio_dev *indio_dev) > goto done; > } > > - ret = taos_i2c_read(chip->client, (TSL258X_CMD_REG), &buf[0], 1); > + ret = i2c_smbus_read_byte_data(chip->client, TSL258X_CMD_REG); > if (ret < 0) { > dev_err(&chip->client->dev, "taos_get_lux failed to read > CMD_REG\n"); > goto done; > } > + > /* is data new & valid */ > - if (!(buf[0] & TSL258X_STA_ADC_INTR)) { > + if (!(ret & TSL258X_STA_ADC_INTR)) { > dev_err(&chip->client->dev, "taos_get_lux data not valid\n"); > ret = chip->als_cur_info.lux; /* return LAST VALUE */ > goto done; > @@ -235,13 +203,14 @@ static int taos_get_lux(struct iio_dev *indio_dev) > for (i = 0; i < 4; i++) { > int reg = TSL258X_CMD_REG | (TSL258X_ALS_CHAN0LO + i); > > - ret = taos_i2c_read(chip->client, reg, &buf[i], 1); > + ret = i2c_smbus_read_byte_data(chip->client, reg); > if (ret < 0) { > dev_err(&chip->client->dev, > "taos_get_lux failed to read register %x\n", > reg); > goto done; > } > + buf[i] = ret; > } > > /* > @@ -343,52 +312,36 @@ static int taos_get_lux(struct iio_dev *indio_dev) > static int taos_als_calibrate(struct iio_dev *indio_dev) > { > struct tsl2583_chip *chip = iio_priv(indio_dev); > - u8 reg_val; > unsigned int gain_trim_val; > int ret; > int lux_val; > > - ret = i2c_smbus_write_byte(chip->client, > -(TSL258X_CMD_REG | TSL258X_CNTRL)); > + ret = i2c_smbus_read_byte_data(chip->clie
Re: [PATCH 2/9] staging: iio: tsl2583: removed unused code from device probing
On 03/11/16 12:56, Brian Masney wrote: > taos_probe() queries the all of the sensor's registers and loads all of > the values into a buffer stored on the stack. Only the chip ID register > was actually used. Change the probe function to just query the chip ID > register on the device. > > Verified that the driver still functions correctly using a TSL2581 > hooked up to a Raspberry Pi 2. > > Signed-off-by: Brian Masney Huh. I'm embarrassed that I didn't notice this bit of silliness in the original reviews :) Applied to the togreg branch of iio.git Thanks, Jonathan > --- > drivers/staging/iio/light/tsl2583.c | 37 > + > 1 file changed, 13 insertions(+), 24 deletions(-) > > diff --git a/drivers/staging/iio/light/tsl2583.c > b/drivers/staging/iio/light/tsl2583.c > index a3a9095..388440b 100644 > --- a/drivers/staging/iio/light/tsl2583.c > +++ b/drivers/staging/iio/light/tsl2583.c > @@ -31,8 +31,6 @@ > #include > #include > > -#define TSL258X_MAX_DEVICE_REGS 32 > - > /* Triton register offsets */ > #define TSL258X_REG_MAX 8 > > @@ -66,6 +64,9 @@ > /* Lux calculation constants */ > #define TSL258X_LUX_CALC_OVER_FLOW 65535 > > +#define TSL2583_CHIP_ID 0x90 > +#define TSL2583_CHIP_ID_MASK 0xf0 > + > enum { > TSL258X_CHIP_UNKNOWN = 0, > TSL258X_CHIP_WORKING = 1, > @@ -607,12 +608,6 @@ static const struct attribute_group > tsl2583_attribute_group = { > .attrs = sysfs_attrs_ctrl, > }; > > -/* Use the default register values to identify the Taos device */ > -static int taos_tsl258x_device(unsigned char *bufp) > -{ > - return ((bufp[TSL258X_CHIPID] & 0xf0) == 0x90); > -} > - > static const struct iio_chan_spec tsl2583_channels[] = { > { > .type = IIO_LIGHT, > @@ -777,8 +772,7 @@ static const struct iio_info tsl2583_info = { > static int taos_probe(struct i2c_client *clientp, > const struct i2c_device_id *idp) > { > - int i, ret; > - unsigned char buf[TSL258X_MAX_DEVICE_REGS]; > + int ret; > struct tsl2583_chip *chip; > struct iio_dev *indio_dev; > > @@ -799,22 +793,17 @@ static int taos_probe(struct i2c_client *clientp, > chip->taos_chip_status = TSL258X_CHIP_UNKNOWN; > memcpy(chip->taos_config, taos_config, sizeof(chip->taos_config)); > > - for (i = 0; i < TSL258X_MAX_DEVICE_REGS; i++) { > - ret = i2c_smbus_read_byte_data(clientp, > -(TSL258X_CMD_REG | > - (TSL258X_CNTRL + i))); > - if (ret < 0) { > - dev_err(&clientp->dev, > - "i2c_smbus_read_byte from reg failed in > taos_probe(), err = %d\n", > - ret); > - return ret; > - } > - buf[i] = ret; > + ret = i2c_smbus_read_byte_data(clientp, > +TSL258X_CMD_REG | TSL258X_CHIPID); > + if (ret < 0) { > + dev_err(&clientp->dev, > + "%s failed to read the chip ID register\n", __func__); > + return ret; > } > > - if (!taos_tsl258x_device(buf)) { > - dev_info(&clientp->dev, > - "i2c device found but does not match expected id in > taos_probe()\n"); > + if ((ret & TSL2583_CHIP_ID_MASK) != TSL2583_CHIP_ID) { > + dev_info(&clientp->dev, "%s received an unknown chip ID %x\n", > + __func__, ret); > return -EINVAL; > } > > ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 3/9] staging: iio: tsl2583: fixed ordering of comments
On 03/11/16 12:56, Brian Masney wrote: > in taos_defaults() > > The comments in taos_defaults() appear after the line of code > that they apply to. This patch moves the comments so that they appear > before the code. Some of the comments were updated to be more > informative. > > Signed-off-by: Brian Masney Nice tidying up. Applied to the togreg branch of iio.git - initially pushed out as testing for the autobuilders to play with it. Thanks, Jonathan > --- > drivers/staging/iio/light/tsl2583.c | 19 +-- > 1 file changed, 13 insertions(+), 6 deletions(-) > > diff --git a/drivers/staging/iio/light/tsl2583.c > b/drivers/staging/iio/light/tsl2583.c > index 388440b..709f446 100644 > --- a/drivers/staging/iio/light/tsl2583.c > +++ b/drivers/staging/iio/light/tsl2583.c > @@ -143,16 +143,23 @@ static const struct gainadj gainadj[] = { > */ > static void taos_defaults(struct tsl2583_chip *chip) > { > - /* Operational parameters */ > + /* > + * The integration time must be a multiple of 50ms and within the > + * range [50, 600] ms. > + */ > chip->taos_settings.als_time = 100; > - /* must be a multiple of 50mS */ > + > + /* > + * This is an index into the gainadj table. Assume clear glass as the > + * default. > + */ > chip->taos_settings.als_gain = 0; > - /* this is actually an index into the gain table */ > - /* assume clear glass as default */ > + > + /* Default gain trim to account for aperture effects */ > chip->taos_settings.als_gain_trim = 1000; > - /* default gain trim to account for aperture effects */ > - chip->taos_settings.als_cal_target = 130; > + > /* Known external ALS reading used for calibration */ > + chip->taos_settings.als_cal_target = 130; > } > > /* > ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 4/9] staging: iio: tsl2583: remove redundant power off sequence in taos_chip_on()
On 03/11/16 12:56, Brian Masney wrote: > taos_chip_on() explicitly turns the sensor power on and then writes the > 8 registers that are stored in taos_config. The first register in > taos_config is the CONTROL register and the configuration is set to > turn the power off. The existing state sequence in taos_chip_on() is: > > - Turn device power on > - Turn device power off (via taos_config) > - Configure other 7 registers (via taos_config) > - Turn device power on, enable ADC > > This patch changes the code so that the device is not powered off via > taos_config. > > Verified that the driver still functions correctly using a TSL2581 > hooked up to a Raspberry Pi 2. > > Signed-off-by: Brian Masney Seems sensible. Applied to the togreg branch of iio.git. Thanks, Jonathan > --- > drivers/staging/iio/light/tsl2583.c | 13 ++--- > 1 file changed, 2 insertions(+), 11 deletions(-) > > diff --git a/drivers/staging/iio/light/tsl2583.c > b/drivers/staging/iio/light/tsl2583.c > index 709f446..7fb09c6 100644 > --- a/drivers/staging/iio/light/tsl2583.c > +++ b/drivers/staging/iio/light/tsl2583.c > @@ -410,17 +410,8 @@ static int taos_chip_on(struct iio_dev *indio_dev) > chip->als_saturation = als_count * 922; /* 90% of full scale */ > chip->als_time_scale = (als_time + 25) / 50; > > - /* > - * TSL258x Specific power-on / adc enable sequence > - * Power on the device 1st. > - */ > - utmp = TSL258X_CNTL_PWR_ON; > - ret = i2c_smbus_write_byte_data(chip->client, > - TSL258X_CMD_REG | TSL258X_CNTRL, utmp); > - if (ret < 0) { > - dev_err(&chip->client->dev, "taos_chip_on failed on CNTRL > reg.\n"); > - return ret; > - } > + /* Power on the device; ADC off. */ > + chip->taos_config[TSL258X_CNTRL] = TSL258X_CNTL_PWR_ON; > > /* >* Use the following shadow copy for our delay before enabling ADC. > ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 5/9] staging: iio: tsl2583: don't shutdown chip when updating the lux table
On 03/11/16 12:56, Brian Masney wrote: > in_illuminance_lux_table_store() shuts down the chip, updates the > contents of the lux table, and then turns the chip back on. The values > in lux table are not used by the chip and are only used internally by > the driver. It is not necessary to change the power state on the chip. > This patch removes the calls to taos_chip_off() and taos_chip_on() > in in_illuminance_lux_table_store(). That is rather mystifying... Maybe a really odd way of locking? > > Signed-off-by: Brian Masney Applied to the togreg branch of iio.git and pushed out as testing. Thanks, Jonathan > --- > drivers/staging/iio/light/tsl2583.c | 10 -- > 1 file changed, 10 deletions(-) > > diff --git a/drivers/staging/iio/light/tsl2583.c > b/drivers/staging/iio/light/tsl2583.c > index 7fb09c6..af1cf9b 100644 > --- a/drivers/staging/iio/light/tsl2583.c > +++ b/drivers/staging/iio/light/tsl2583.c > @@ -564,20 +564,10 @@ static ssize_t in_illuminance_lux_table_store(struct > device *dev, > goto done; > } > > - if (chip->taos_chip_status == TSL258X_CHIP_WORKING) { > - ret = taos_chip_off(indio_dev); > - if (ret < 0) > - goto done; > - } > - > /* Zero out the table */ > memset(taos_device_lux, 0, sizeof(taos_device_lux)); > memcpy(taos_device_lux, &value[1], (value[0] * 4)); > > - ret = taos_chip_on(indio_dev); > - if (ret < 0) > - goto done; > - > ret = len; > > done: > ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 6/9] staging: iio: tsl2583: remove redudant i2c call in taos_als_calibrate()
On 03/11/16 12:56, Brian Masney wrote: > taos_als_calibrate() queries the control register to determine if the > unit is powered on and has the ADC enabled. It then queries the same > register a second time to determine if the ADC reading is valid. This > patch removes the redundant i2c_smbus_read_byte_data() call. > > Verified that the driver still functions correctly using a TSL2581 > hooked up to a Raspberry Pi 2. > > Signed-off-by: Brian Masney Applied. Thanks, Jonathan > --- > drivers/staging/iio/light/tsl2583.c | 13 + > 1 file changed, 1 insertion(+), 12 deletions(-) > > diff --git a/drivers/staging/iio/light/tsl2583.c > b/drivers/staging/iio/light/tsl2583.c > index af1cf9b..7eab17f 100644 > --- a/drivers/staging/iio/light/tsl2583.c > +++ b/drivers/staging/iio/light/tsl2583.c > @@ -338,18 +338,7 @@ static int taos_als_calibrate(struct iio_dev *indio_dev) > dev_err(&chip->client->dev, > "taos_als_calibrate failed: device not powered on with > ADC enabled\n"); > return -EINVAL; > - } > - > - ret = i2c_smbus_read_byte_data(chip->client, > -TSL258X_CMD_REG | TSL258X_CNTRL); > - if (ret < 0) { > - dev_err(&chip->client->dev, > - "%s failed to read from the CNTRL register\n", > - __func__); > - return ret; > - } > - > - if ((ret & TSL258X_STA_ADC_VALID) != TSL258X_STA_ADC_VALID) { > + } else if ((ret & TSL258X_STA_ADC_VALID) != TSL258X_STA_ADC_VALID) { > dev_err(&chip->client->dev, > "taos_als_calibrate failed: STATUS - ADC not valid.\n"); > return -ENODATA; > ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 7/9] staging: iio: tsl2583: fix issue with changes to calibscale and int_time not being set on the chip
On 03/11/16 12:56, Brian Masney wrote: > When updating the in_illuminance_calibscale and > in_illuminance_integration_time sysfs attributes, these values were not > actually written to the chip. The chip would continue to use the old > parameters. Extracted out tsl2583_set_als_gain() and > tsl2583_set_als_time() functions that are now called when these sysfs > attributes are updated. The chip initialization now calls these these > new functions. > > Rename taos_chip_on() to tsl2583_chip_init() since it is now only called > during device probing and when the power management code wakes the > device back up. tsl2583_chip_init() was refactored to use the new > functions mentioned above. > > Previously, the current chip state was represented as a tristate > (working, suspended, and unknown). The unknown state was not used. The > chip state is now represented with a single boolean value (suspended). Last part should probably have been a separate patch. Earlier stages could also have been futher broken up I think to make it easier to review. The additional init in the resume path should also protect against suspends which actually cut the power to the chip which is nice. Just enough bits and pieces inline that I'd like you to do another pass on this. Jonathan > > Signed-off-by: Brian Masney > --- > drivers/staging/iio/light/tsl2583.c | 172 > > 1 file changed, 78 insertions(+), 94 deletions(-) > > diff --git a/drivers/staging/iio/light/tsl2583.c > b/drivers/staging/iio/light/tsl2583.c > index 7eab17f..1ff90b3 100644 > --- a/drivers/staging/iio/light/tsl2583.c > +++ b/drivers/staging/iio/light/tsl2583.c > @@ -48,6 +48,8 @@ > #define TSL258X_TMR_LO 0x18 > #define TSL258X_TMR_HI 0x19 > > +#define TSL2583_INTERRUPT_DISABLED 0x00 > + > /* tsl2583 cmd reg masks */ > #define TSL258X_CMD_REG 0x80 > #define TSL258X_CMD_SPL_FN 0x60 > @@ -55,6 +57,7 @@ > > /* tsl2583 cntrl reg masks */ > #define TSL258X_CNTL_ADC_ENBL0x02 > +#define TSL258X_CNTL_PWR_OFF 0x00 > #define TSL258X_CNTL_PWR_ON 0x01 > > /* tsl2583 status reg masks */ > @@ -67,12 +70,6 @@ > #define TSL2583_CHIP_ID 0x90 > #define TSL2583_CHIP_ID_MASK 0xf0 > > -enum { > - TSL258X_CHIP_UNKNOWN = 0, > - TSL258X_CHIP_WORKING = 1, > - TSL258X_CHIP_SUSPENDED = 2 > -}; > - > /* Per-device data */ > struct taos_als_info { > u16 als_ch0; > @@ -94,19 +91,9 @@ struct tsl2583_chip { > struct taos_settings taos_settings; > int als_time_scale; > int als_saturation; > - int taos_chip_status; > - u8 taos_config[8]; > + bool suspended; > }; > > -/* > - * Initial values for device - this values can/will be changed by driver. > - * and applications as needed. > - * These values are dynamic. > - */ > -static const u8 taos_config[8] = { > - 0x00, 0xee, 0x00, 0x03, 0x00, 0xFF, 0xFF, 0x00 > -}; /*cntrl atime intC Athl0 Athl1 Athh0 Athh1 gain */ > - > struct taos_lux { > unsigned int ratio; > unsigned int ch0; > @@ -188,13 +175,6 @@ static int taos_get_lux(struct iio_dev *indio_dev) > u32 ch0lux = 0; > u32 ch1lux = 0; > > - if (chip->taos_chip_status != TSL258X_CHIP_WORKING) { > - /* device is not enabled */ > - dev_err(&chip->client->dev, "taos_get_lux device is not > enabled\n"); > - ret = -EBUSY; > - goto done; > - } > - This change is because it's already guaranteed to be in the working state? > ret = i2c_smbus_read_byte_data(chip->client, TSL258X_CMD_REG); > if (ret < 0) { > dev_err(&chip->client->dev, "taos_get_lux failed to read > CMD_REG\n"); > @@ -362,26 +342,10 @@ static int taos_als_calibrate(struct iio_dev *indio_dev) > return (int)gain_trim_val; > } > > -/* > - * Turn the device on. > - * Configuration must be set before calling this function. > - */ > -static int taos_chip_on(struct iio_dev *indio_dev) > +static int tsl2583_set_als_time(struct tsl2583_chip *chip) > { > - int i; > - int ret; > - u8 *uP; > - u8 utmp; > - int als_count; > - int als_time; > - struct tsl2583_chip *chip = iio_priv(indio_dev); > - > - /* and make sure we're not already on */ > - if (chip->taos_chip_status == TSL258X_CHIP_WORKING) { > - /* if forcing a register update - turn off, then on */ > - dev_info(&chip->client->dev, "device is already enabled\n"); > - return -EINVAL; > - } > + int als_count, als_time, ret; > + u8 val; > > /* determine als integration register */ > als_count = (chip->taos_settings.als_time * 100 + 135) / 270; > @@ -390,60 +354,82 @@ static int taos_chip_on(struct iio_dev *indio_dev) > > /* convert back to time (encompasses overrides) */ > als_time = (als_count * 27 + 5) / 10; > -
Re: [PATCH 8/9] staging: iio: tsl2583: check if chip is suspended in in_illuminance_calibrate_store
On 03/11/16 12:56, Brian Masney wrote: > in_illuminance_calibrate_store() did not check to see if the chip is > suspended. This patch adds the proper check. The return value from > taos_als_calibrate() was also not checked in this function, so the > proper check was also added while changes are being made here. > > Signed-off-by: Brian Masney This one is fine, I'll pick up after patch 7 stuff is sorted. Jonathan > --- > drivers/staging/iio/light/tsl2583.c | 17 ++--- > 1 file changed, 14 insertions(+), 3 deletions(-) > > diff --git a/drivers/staging/iio/light/tsl2583.c > b/drivers/staging/iio/light/tsl2583.c > index 1ff90b3..8c8361d 100644 > --- a/drivers/staging/iio/light/tsl2583.c > +++ b/drivers/staging/iio/light/tsl2583.c > @@ -473,16 +473,27 @@ static ssize_t in_illuminance_calibrate_store(struct > device *dev, > { > struct iio_dev *indio_dev = dev_to_iio_dev(dev); > struct tsl2583_chip *chip = iio_priv(indio_dev); > - int value; > + int value, ret; > > if (kstrtoint(buf, 0, &value) || value != 1) > return -EINVAL; > > mutex_lock(&chip->als_mutex); > - taos_als_calibrate(indio_dev); > + > + if (chip->suspended) { > + ret = -EBUSY; > + goto done; > + } > + > + ret = taos_als_calibrate(indio_dev); > + if (ret < 0) > + goto done; > + > + ret = len; > +done: > mutex_unlock(&chip->als_mutex); > > - return len; > + return ret; > } > > static ssize_t in_illuminance_lux_table_show(struct device *dev, > ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 9/9] staging: iio: tsl2583: remove redundant write to the control register in taos_probe()
On 03/11/16 12:56, Brian Masney wrote: > taos_probe() calls i2c_smbus_write_byte() to select the control > register, however there is no subsequent calls to i2c_smbus_read_byte(). > The write call is unnecessary and is removed by this patch. > > Verified that the driver still functions correctly using a TSL2581 > hooked up to a Raspberry Pi 2. > > Signed-off-by: Brian Masney Again, looks fine - will pickup when precursors are sorted. Another generally nice series Brian. Thanks! Jonathan > --- > drivers/staging/iio/light/tsl2583.c | 8 > 1 file changed, 8 deletions(-) > > diff --git a/drivers/staging/iio/light/tsl2583.c > b/drivers/staging/iio/light/tsl2583.c > index 8c8361d..e80c027 100644 > --- a/drivers/staging/iio/light/tsl2583.c > +++ b/drivers/staging/iio/light/tsl2583.c > @@ -781,14 +781,6 @@ static int taos_probe(struct i2c_client *clientp, > return -EINVAL; > } > > - ret = i2c_smbus_write_byte(clientp, (TSL258X_CMD_REG | TSL258X_CNTRL)); > - if (ret < 0) { > - dev_err(&clientp->dev, > - "i2c_smbus_write_byte() to cmd reg failed in > taos_probe(), err = %d\n", > - ret); > - return ret; > - } > - > indio_dev->info = &tsl2583_info; > indio_dev->channels = tsl2583_channels; > indio_dev->num_channels = ARRAY_SIZE(tsl2583_channels); > ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH] Staging: speakup: Remove unnecessary space after cast
The affected files have been modified to remove redundant spaces after casts to solve checkpatch.pl checks. Signed-off-by: Shiva Kerdel --- drivers/staging/speakup/main.c | 42 ++--- drivers/staging/speakup/selection.c | 2 +- drivers/staging/speakup/serialio.c | 6 +++--- 3 files changed, 25 insertions(+), 25 deletions(-) diff --git a/drivers/staging/speakup/main.c b/drivers/staging/speakup/main.c index 97ca4ec..5c19204 100644 --- a/drivers/staging/speakup/main.c +++ b/drivers/staging/speakup/main.c @@ -351,14 +351,14 @@ static void speakup_cut(struct vc_data *vc) if (!mark_cut_flag) { mark_cut_flag = 1; - spk_xs = (u_short) spk_x; - spk_ys = (u_short) spk_y; + spk_xs = (u_short)spk_x; + spk_ys = (u_short)spk_y; spk_sel_cons = vc; synth_printf("%s\n", spk_msg_get(MSG_MARK)); return; } - spk_xe = (u_short) spk_x; - spk_ye = (u_short) spk_y; + spk_xe = (u_short)spk_x; + spk_ye = (u_short)spk_y; mark_cut_flag = 0; synth_printf("%s\n", spk_msg_get(MSG_CUT)); @@ -489,7 +489,7 @@ static void say_char(struct vc_data *vc) u_short ch; spk_old_attr = spk_attr; - ch = get_char(vc, (u_short *) spk_pos, &spk_attr); + ch = get_char(vc, (u_short *)spk_pos, &spk_attr); if (spk_attr != spk_old_attr) { if (spk_attrib_bleep & 1) bleep(spk_y); @@ -504,7 +504,7 @@ static void say_phonetic_char(struct vc_data *vc) u_short ch; spk_old_attr = spk_attr; - ch = get_char(vc, (u_short *) spk_pos, &spk_attr); + ch = get_char(vc, (u_short *)spk_pos, &spk_attr); if (isascii(ch) && isalpha(ch)) { ch &= 0x1f; synth_printf("%s\n", phonetic[--ch]); @@ -556,7 +556,7 @@ static u_long get_word(struct vc_data *vc) u_char temp; spk_old_attr = spk_attr; - ch = (char)get_char(vc, (u_short *) tmp_pos, &temp); + ch = (char)get_char(vc, (u_short *)tmp_pos, &temp); /* decided to take out the sayword if on a space (mis-information */ if (spk_say_word_ctl && ch == SPACE) { @@ -565,26 +565,26 @@ static u_long get_word(struct vc_data *vc) return 0; } else if ((tmpx < vc->vc_cols - 2) && (ch == SPACE || ch == 0 || IS_WDLM(ch)) - && ((char)get_char(vc, (u_short *) &tmp_pos + 1, &temp) > + && ((char)get_char(vc, (u_short *)&tmp_pos + 1, &temp) > SPACE)) { tmp_pos += 2; tmpx++; } else while (tmpx > 0) { - ch = (char)get_char(vc, (u_short *) tmp_pos - 1, &temp); + ch = (char)get_char(vc, (u_short *)tmp_pos - 1, &temp); if ((ch == SPACE || ch == 0 || IS_WDLM(ch)) - && ((char)get_char(vc, (u_short *) tmp_pos, &temp) > + && ((char)get_char(vc, (u_short *)tmp_pos, &temp) > SPACE)) break; tmp_pos -= 2; tmpx--; } - attr_ch = get_char(vc, (u_short *) tmp_pos, &spk_attr); + attr_ch = get_char(vc, (u_short *)tmp_pos, &spk_attr); buf[cnt++] = attr_ch & 0xff; while (tmpx < vc->vc_cols - 1) { tmp_pos += 2; tmpx++; - ch = (char)get_char(vc, (u_short *) tmp_pos, &temp); + ch = (char)get_char(vc, (u_short *)tmp_pos, &temp); if ((ch == SPACE) || ch == 0 || (IS_WDLM(buf[cnt - 1]) && (ch > SPACE))) break; @@ -639,7 +639,7 @@ static void say_prev_word(struct vc_data *vc) } else spk_x--; spk_pos -= 2; - ch = (char)get_char(vc, (u_short *) spk_pos, &temp); + ch = (char)get_char(vc, (u_short *)spk_pos, &temp); if (ch == SPACE || ch == 0) state = 0; else if (IS_WDLM(ch)) @@ -672,7 +672,7 @@ static void say_next_word(struct vc_data *vc) return; } while (1) { - ch = (char)get_char(vc, (u_short *) spk_pos, &temp); + ch = (char)get_char(vc, (u_short *)spk_pos, &temp); if (ch == SPACE || ch == 0) state = 0; else if (IS_WDLM(ch)) @@ -709,7 +709,7 @@ static void spell_word(struct vc_data *vc) if (!get_word(vc)) return; - while ((ch = (u_char) *cp)) { + while ((ch = (u_char)*cp)) { if (cp != buf) synth_printf(" %s ", delay_str[spk_spell_delay]); if (IS_CHAR(ch, B_CAP)) { @@ -751,7 +751,7 @@ static in
Re: [PATCH 7/9] staging: iio: tsl2583: fix issue with changes to calibscale and int_time not being set on the chip
On Sun, Nov 06, 2016 at 12:03:53PM +, Jonathan Cameron wrote: > On 03/11/16 12:56, Brian Masney wrote: > > When updating the in_illuminance_calibscale and > > in_illuminance_integration_time sysfs attributes, these values were not > > actually written to the chip. The chip would continue to use the old > > parameters. Extracted out tsl2583_set_als_gain() and > > tsl2583_set_als_time() functions that are now called when these sysfs > > attributes are updated. The chip initialization now calls these these > > new functions. > > > > Rename taos_chip_on() to tsl2583_chip_init() since it is now only called > > during device probing and when the power management code wakes the > > device back up. tsl2583_chip_init() was refactored to use the new > > functions mentioned above. > > > > Previously, the current chip state was represented as a tristate > > (working, suspended, and unknown). The unknown state was not used. The > > chip state is now represented with a single boolean value (suspended). > Last part should probably have been a separate patch. Earlier stages could > also have been futher broken up I think to make it easier to review. > > The additional init in the resume path should also protect against suspends > which actually cut the power to the chip which is nice. > > Just enough bits and pieces inline that I'd like you to do another pass > on this. No problem, I'll split this one up for you. My next patch series will also contain a lot of trivial code cleanups, some documentation updates, and a request to move the driver out of staging. The device tree documentation (Documentation/devicetree/bindings/iio/light/tsl2583.txt) has the interrupt-parent and interrupts properties as optional, however the driver does not support interrupts. Should I remove these properties from the device tree documentation? I can add the code to support the interrupts but I am hesistant to add that new code if no one will use it. Brian ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH 1/2] staging: lustre: obdclass: Fix error handling
'lustre_cfg_new()' does not return NULL but ERR_PTR(-ENOMEM) So IS_ERR should be used to check the return value. Signed-off-by: Christophe JAILLET --- drivers/staging/lustre/lustre/obdclass/obd_config.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/staging/lustre/lustre/obdclass/obd_config.c b/drivers/staging/lustre/lustre/obdclass/obd_config.c index bbed1b72d52e..0e922c5f8b33 100644 --- a/drivers/staging/lustre/lustre/obdclass/obd_config.c +++ b/drivers/staging/lustre/lustre/obdclass/obd_config.c @@ -1382,8 +1382,8 @@ int class_manual_cleanup(struct obd_device *obd) lustre_cfg_bufs_reset(&bufs, obd->obd_name); lustre_cfg_bufs_set_string(&bufs, 1, flags); lcfg = lustre_cfg_new(LCFG_CLEANUP, &bufs); - if (!lcfg) - return -ENOMEM; + if (IS_ERR(lcfg)) + return PTR_ERR(lcfg); rc = class_process_config(lcfg); if (rc) { -- 2.9.3 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH 2/2] staging: lustre: obdclass: Add handling of error returned by lustre_cfg_new
'lustre_cfg_new()' can return ERR_PTR(-ENOMEM). Handle these errors and propagate the error code to the callers. Error handling has been rearranged in 'lustre_process_log()' with the addition of a label in order to free some resources. Signed-off-by: Christophe JAILLET --- drivers/staging/lustre/lustre/obdclass/obd_mount.c | 16 ++-- 1 file changed, 14 insertions(+), 2 deletions(-) diff --git a/drivers/staging/lustre/lustre/obdclass/obd_mount.c b/drivers/staging/lustre/lustre/obdclass/obd_mount.c index 59fbc29aae94..5473615cd338 100644 --- a/drivers/staging/lustre/lustre/obdclass/obd_mount.c +++ b/drivers/staging/lustre/lustre/obdclass/obd_mount.c @@ -89,11 +89,14 @@ int lustre_process_log(struct super_block *sb, char *logname, lustre_cfg_bufs_set(bufs, 2, cfg, sizeof(*cfg)); lustre_cfg_bufs_set(bufs, 3, &sb, sizeof(sb)); lcfg = lustre_cfg_new(LCFG_LOG_START, bufs); + if (IS_ERR(lcfg)) { + rc = PTR_ERR(lcfg); + goto out_free; + } + rc = obd_process_config(mgc, sizeof(*lcfg), lcfg); lustre_cfg_free(lcfg); - kfree(bufs); - if (rc == -EINVAL) LCONSOLE_ERROR_MSG(0x15b, "%s: The configuration from log '%s' failed from the MGS (%d). Make sure this client and the MGS are running compatible versions of Lustre.\n", mgc->obd_name, logname, rc); @@ -104,6 +107,9 @@ int lustre_process_log(struct super_block *sb, char *logname, rc); /* class_obd_list(); */ + +out_free: + kfree(bufs); return rc; } EXPORT_SYMBOL(lustre_process_log); @@ -127,6 +133,9 @@ int lustre_end_log(struct super_block *sb, char *logname, if (cfg) lustre_cfg_bufs_set(&bufs, 2, cfg, sizeof(*cfg)); lcfg = lustre_cfg_new(LCFG_LOG_END, &bufs); + if (IS_ERR(lcfg)) + return PTR_ERR(lcfg); + rc = obd_process_config(mgc, sizeof(*lcfg), lcfg); lustre_cfg_free(lcfg); return rc; @@ -159,6 +168,9 @@ static int do_lcfg(char *cfgname, lnet_nid_t nid, int cmd, lustre_cfg_bufs_set_string(&bufs, 4, s4); lcfg = lustre_cfg_new(cmd, &bufs); + if (IS_ERR(lcfg)) + return PTR_ERR(lcfg); + lcfg->lcfg_nid = nid; rc = class_process_config(lcfg); lustre_cfg_free(lcfg); -- 2.9.3 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 2/2] staging: lustre: obdclass: Add handling of error returned by lustre_cfg_new
Hello! On Nov 6, 2016, at 12:11 PM, Christophe JAILLET wrote: > 'lustre_cfg_new()' can return ERR_PTR(-ENOMEM). > Handle these errors and propagate the error code to the callers. > > Error handling has been rearranged in 'lustre_process_log()' with the > addition of a label in order to free some resources. I wonder if we should just make it return NULL on allocation failure, and then at least the other error handling that is there (i.e. in your other patch) would become correct. This would make handling in mgc_apply_recover_logs incorrect, but it's already geared towards this sort of handling anyway, as it discards the passed error and sets ENOMEM unconditionally (just need to revert 3092c34a in a way). Thanks! > > Signed-off-by: Christophe JAILLET > --- > drivers/staging/lustre/lustre/obdclass/obd_mount.c | 16 ++-- > 1 file changed, 14 insertions(+), 2 deletions(-) > > diff --git a/drivers/staging/lustre/lustre/obdclass/obd_mount.c > b/drivers/staging/lustre/lustre/obdclass/obd_mount.c > index 59fbc29aae94..5473615cd338 100644 > --- a/drivers/staging/lustre/lustre/obdclass/obd_mount.c > +++ b/drivers/staging/lustre/lustre/obdclass/obd_mount.c > @@ -89,11 +89,14 @@ int lustre_process_log(struct super_block *sb, char > *logname, > lustre_cfg_bufs_set(bufs, 2, cfg, sizeof(*cfg)); > lustre_cfg_bufs_set(bufs, 3, &sb, sizeof(sb)); > lcfg = lustre_cfg_new(LCFG_LOG_START, bufs); > + if (IS_ERR(lcfg)) { > + rc = PTR_ERR(lcfg); > + goto out_free; > + } > + > rc = obd_process_config(mgc, sizeof(*lcfg), lcfg); > lustre_cfg_free(lcfg); > > - kfree(bufs); > - > if (rc == -EINVAL) > LCONSOLE_ERROR_MSG(0x15b, "%s: The configuration from log '%s' > failed from the MGS (%d). Make sure this client and the MGS are running > compatible versions of Lustre.\n", > mgc->obd_name, logname, rc); > @@ -104,6 +107,9 @@ int lustre_process_log(struct super_block *sb, char > *logname, > rc); > > /* class_obd_list(); */ > + > +out_free: > + kfree(bufs); > return rc; > } > EXPORT_SYMBOL(lustre_process_log); > @@ -127,6 +133,9 @@ int lustre_end_log(struct super_block *sb, char *logname, > if (cfg) > lustre_cfg_bufs_set(&bufs, 2, cfg, sizeof(*cfg)); > lcfg = lustre_cfg_new(LCFG_LOG_END, &bufs); > + if (IS_ERR(lcfg)) > + return PTR_ERR(lcfg); > + > rc = obd_process_config(mgc, sizeof(*lcfg), lcfg); > lustre_cfg_free(lcfg); > return rc; > @@ -159,6 +168,9 @@ static int do_lcfg(char *cfgname, lnet_nid_t nid, int cmd, > lustre_cfg_bufs_set_string(&bufs, 4, s4); > > lcfg = lustre_cfg_new(cmd, &bufs); > + if (IS_ERR(lcfg)) > + return PTR_ERR(lcfg); > + > lcfg->lcfg_nid = nid; > rc = class_process_config(lcfg); > lustre_cfg_free(lcfg); > -- > 2.9.3 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 7/9] staging: iio: tsl2583: fix issue with changes to calibscale and int_time not being set on the chip
On 06/11/16 14:23, Brian Masney wrote: > On Sun, Nov 06, 2016 at 12:03:53PM +, Jonathan Cameron wrote: >> On 03/11/16 12:56, Brian Masney wrote: >>> When updating the in_illuminance_calibscale and >>> in_illuminance_integration_time sysfs attributes, these values were not >>> actually written to the chip. The chip would continue to use the old >>> parameters. Extracted out tsl2583_set_als_gain() and >>> tsl2583_set_als_time() functions that are now called when these sysfs >>> attributes are updated. The chip initialization now calls these these >>> new functions. >>> >>> Rename taos_chip_on() to tsl2583_chip_init() since it is now only called >>> during device probing and when the power management code wakes the >>> device back up. tsl2583_chip_init() was refactored to use the new >>> functions mentioned above. >>> >>> Previously, the current chip state was represented as a tristate >>> (working, suspended, and unknown). The unknown state was not used. The >>> chip state is now represented with a single boolean value (suspended). >> Last part should probably have been a separate patch. Earlier stages could >> also have been futher broken up I think to make it easier to review. >> >> The additional init in the resume path should also protect against suspends >> which actually cut the power to the chip which is nice. >> >> Just enough bits and pieces inline that I'd like you to do another pass >> on this. > > No problem, I'll split this one up for you. My next patch series will > also contain a lot of trivial code cleanups, some documentation updates, and > a request to move the driver out of staging. Cool > > The device tree documentation > (Documentation/devicetree/bindings/iio/light/tsl2583.txt) has the > interrupt-parent and interrupts properties as optional, however the > driver does not support interrupts. Should I remove these properties from > the device tree documentation? I can add the code to support the > interrupts but I am hesistant to add that new code if no one will use it. > > Brian > Leave them there. Device tree is about documenting the hardware, not just those bits we get around to using ;) Obviously we have to document what we use but no problem documenting other bits! Jonathan ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH V3 00/14] Drivers: hv: Some miscellaneous fixes and enhancements
Some miscellaneous fixes and enhancements. V2: Fixed a build issue reported by Greg. Only Patch # 13 is affected. V3: Address Stephen's comment. Only patch 3 is affected. Alex Ng (6): Drivers: hv: utils: Fix the mapping between host version and protocol to use Drivers: hv: balloon: Disable hot add when CONFIG_MEMORY_HOTPLUG is not set Drivers: hv: balloon: Add logging for dynamic memory operations Drivers: hv: vss: Improve log messages. Drivers: hv: vss: Operation timeouts should match host expectation Drivers: hv: balloon: Fix info request to show max page count K. Y. Srinivasan (3): Drivers: hv: vmbus: Base host signaling strictly on the ring state Drivers: hv: vmbus: On write cleanup the logic to interrupt the host Drivers: hv: vmbus: On the read path cleanup the logic to interrupt the host Vitaly Kuznetsov (2): Drivers: hv: ring_buffer: count on wrap around mappings in get_next_pkt_raw() (v2) Drivers: hv: utils: reduce HV_UTIL_NEGO_TIMEOUT timeout Weibing Zhang (3): tools: hv: remove unnecessary link flag tools: hv: fix a compile warning in snprintf tools: hv: remove unnecessary header files and netlink related code drivers/hv/channel.c | 93 ++-- drivers/hv/channel_mgmt.c |2 - drivers/hv/hv_balloon.c| 44 ++-- drivers/hv/hv_snapshot.c | 33 drivers/hv/hv_util.c |9 +++- drivers/hv/hyperv_vmbus.h | 12 +++--- drivers/hv/ring_buffer.c | 44 - include/linux/hyperv.h | 45 - tools/hv/Makefile |3 +- tools/hv/hv_fcopy_daemon.c |7 --- tools/hv/hv_kvp_daemon.c |9 + 11 files changed, 133 insertions(+), 168 deletions(-) -- 1.7.4.1 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH V3 00/14] Drivers: hv: Some miscellaneous fixes and enhancements
From: K. Y. Srinivasan Some miscellaneous fixes and enhancements. V2: Fixed a build issue reported by Greg. Only Patch # 13 is affected. V3: Address Stephen's comment. Only patch 3 is affected. Alex Ng (6): Drivers: hv: utils: Fix the mapping between host version and protocol to use Drivers: hv: balloon: Disable hot add when CONFIG_MEMORY_HOTPLUG is not set Drivers: hv: balloon: Add logging for dynamic memory operations Drivers: hv: vss: Improve log messages. Drivers: hv: vss: Operation timeouts should match host expectation Drivers: hv: balloon: Fix info request to show max page count K. Y. Srinivasan (3): Drivers: hv: vmbus: Base host signaling strictly on the ring state Drivers: hv: vmbus: On write cleanup the logic to interrupt the host Drivers: hv: vmbus: On the read path cleanup the logic to interrupt the host Vitaly Kuznetsov (2): Drivers: hv: ring_buffer: count on wrap around mappings in get_next_pkt_raw() (v2) Drivers: hv: utils: reduce HV_UTIL_NEGO_TIMEOUT timeout Weibing Zhang (3): tools: hv: remove unnecessary link flag tools: hv: fix a compile warning in snprintf tools: hv: remove unnecessary header files and netlink related code drivers/hv/channel.c | 93 ++-- drivers/hv/channel_mgmt.c |2 - drivers/hv/hv_balloon.c| 44 ++-- drivers/hv/hv_snapshot.c | 33 drivers/hv/hv_util.c |9 +++- drivers/hv/hyperv_vmbus.h | 12 +++--- drivers/hv/ring_buffer.c | 44 - include/linux/hyperv.h | 45 - tools/hv/Makefile |3 +- tools/hv/hv_fcopy_daemon.c |7 --- tools/hv/hv_kvp_daemon.c |9 + 11 files changed, 133 insertions(+), 168 deletions(-) -- 1.7.4.1 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH V3 12/14] Drivers: hv: vmbus: Base host signaling strictly on the ring state
From: K. Y. Srinivasan One of the factors that can result in the host concluding that a given guest in mounting a DOS attack is if the guest generates interrupts to the host when the host is not expecting it. If these "spurious" interrupts reach a certain rate, the host can throttle the guest to minimize the impact. The host computation of the "expected number of interrupts" is strictly based on the ring transitions. Until the host logic is fixed, base the guest logic to interrupt solely on the ring state. Signed-off-by: K. Y. Srinivasan --- drivers/hv/channel.c | 23 --- drivers/hv/channel_mgmt.c |2 -- drivers/hv/ring_buffer.c |7 --- 3 files changed, 20 insertions(+), 12 deletions(-) diff --git a/drivers/hv/channel.c b/drivers/hv/channel.c index 16f91c8..5e482d7 100644 --- a/drivers/hv/channel.c +++ b/drivers/hv/channel.c @@ -676,10 +676,18 @@ int vmbus_sendpacket_ctl(struct vmbus_channel *channel, void *buffer, * NOTE: in this case, the hvsock channel is an exception, because * it looks the host side's hvsock implementation has a throttling * mechanism which can hurt the performance otherwise. +* +* KYS: Oct. 30, 2016: +* It looks like Windows hosts have logic to deal with DOS attacks that +* can be triggered if it receives interrupts when it is not expecting +* the interrupt. The host expects interrupts only when the ring +* transitions from empty to non-empty (or full to non full on the guest +* to host ring). +* So, base the signaling decision solely on the ring state until the +* host logic is fixed. */ - if (((ret == 0) && kick_q && signal) || - (ret && !is_hvsock_channel(channel))) + if (((ret == 0) && signal)) vmbus_setevent(channel); return ret; @@ -786,9 +794,18 @@ int vmbus_sendpacket_pagebuffer_ctl(struct vmbus_channel *channel, * If we cannot write to the ring-buffer; signal the host * even if we may not have written anything. This is a rare * enough condition that it should not matter. +* +* KYS: Oct. 30, 2016: +* It looks like Windows hosts have logic to deal with DOS attacks that +* can be triggered if it receives interrupts when it is not expecting +* the interrupt. The host expects interrupts only when the ring +* transitions from empty to non-empty (or full to non full on the guest +* to host ring). +* So, base the signaling decision solely on the ring state until the +* host logic is fixed. */ - if (((ret == 0) && kick_q && signal) || (ret)) + if (((ret == 0) && signal)) vmbus_setevent(channel); return ret; diff --git a/drivers/hv/channel_mgmt.c b/drivers/hv/channel_mgmt.c index 96a85cd..cbb96f2 100644 --- a/drivers/hv/channel_mgmt.c +++ b/drivers/hv/channel_mgmt.c @@ -447,8 +447,6 @@ static void vmbus_process_offer(struct vmbus_channel *newchannel) } dev_type = hv_get_dev_type(newchannel); - if (dev_type == HV_NIC) - set_channel_signal_state(newchannel, HV_SIGNAL_POLICY_EXPLICIT); init_vp_index(newchannel, dev_type); diff --git a/drivers/hv/ring_buffer.c b/drivers/hv/ring_buffer.c index 08043da..5d11d93 100644 --- a/drivers/hv/ring_buffer.c +++ b/drivers/hv/ring_buffer.c @@ -75,13 +75,6 @@ static bool hv_need_to_signal(u32 old_write, struct hv_ring_buffer_info *rbi, if (READ_ONCE(rbi->ring_buffer->interrupt_mask)) return false; - /* -* When the client wants to control signaling, -* we only honour the host interrupt mask. -*/ - if (policy == HV_SIGNAL_POLICY_EXPLICIT) - return true; - /* check interrupt_mask before read_index */ virt_rmb(); /* -- 1.7.4.1 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH V3 11/14] tools: hv: remove unnecessary header files and netlink related code
From: Weibing Zhang Remove unnecessary header files and netlink related code as the daemons do not use netlink to communicate with the kernel now. Signed-off-by: Weibing Zhang Signed-off-by: K. Y. Srinivasan --- tools/hv/hv_fcopy_daemon.c |7 --- tools/hv/hv_kvp_daemon.c |7 --- 2 files changed, 0 insertions(+), 14 deletions(-) diff --git a/tools/hv/hv_fcopy_daemon.c b/tools/hv/hv_fcopy_daemon.c index fdc9ca4..26ae609 100644 --- a/tools/hv/hv_fcopy_daemon.c +++ b/tools/hv/hv_fcopy_daemon.c @@ -18,21 +18,14 @@ #include -#include -#include -#include -#include #include #include #include -#include -#include #include #include #include #include #include -#include #include static int target_fd; diff --git a/tools/hv/hv_kvp_daemon.c b/tools/hv/hv_kvp_daemon.c index bb0719b..d791dbf 100644 --- a/tools/hv/hv_kvp_daemon.c +++ b/tools/hv/hv_kvp_daemon.c @@ -22,8 +22,6 @@ */ -#include -#include #include #include #include @@ -34,7 +32,6 @@ #include #include #include -#include #include #include #include @@ -99,10 +96,6 @@ enum { #define MAX_FILE_NAME 100 #define ENTRIES_PER_BLOCK 50 -#ifndef SOL_NETLINK -#define SOL_NETLINK 270 -#endif - struct kvp_record { char key[HV_KVP_EXCHANGE_MAX_KEY_SIZE]; char value[HV_KVP_EXCHANGE_MAX_VALUE_SIZE]; -- 1.7.4.1 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH V3 02/14] Drivers: hv: utils: reduce HV_UTIL_NEGO_TIMEOUT timeout
From: Vitaly Kuznetsov I discovered that at least WS2016TP5 host has 60 seconds timeout for the ICMSGTYPE_NEGOTIATE message so we need to lower guest's timeout a little bit to make sure we always respond in time. Let's make it 55 seconds. Signed-off-by: Vitaly Kuznetsov Signed-off-by: K. Y. Srinivasan --- drivers/hv/hyperv_vmbus.h |2 +- 1 files changed, 1 insertions(+), 1 deletions(-) diff --git a/drivers/hv/hyperv_vmbus.h b/drivers/hv/hyperv_vmbus.h index a5b4442..bdab7e7 100644 --- a/drivers/hv/hyperv_vmbus.h +++ b/drivers/hv/hyperv_vmbus.h @@ -38,7 +38,7 @@ /* * Timeout for guest-host handshake for services. */ -#define HV_UTIL_NEGO_TIMEOUT 60 +#define HV_UTIL_NEGO_TIMEOUT 55 /* * The below CPUID leaves are present if VersionAndFeatures.HypervisorPresent -- 1.7.4.1 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH V3 09/14] tools: hv: remove unnecessary link flag
From: Weibing Zhang The link flag pthread is not needed. Signed-off-by: Weibing Zhang Signed-off-by: K. Y. Srinivasan --- tools/hv/Makefile |3 +-- 1 files changed, 1 insertions(+), 2 deletions(-) diff --git a/tools/hv/Makefile b/tools/hv/Makefile index a8c4644..0d1e61b 100644 --- a/tools/hv/Makefile +++ b/tools/hv/Makefile @@ -1,9 +1,8 @@ # Makefile for Hyper-V tools CC = $(CROSS_COMPILE)gcc -PTHREAD_LIBS = -lpthread WARNINGS = -Wall -Wextra -CFLAGS = $(WARNINGS) -g $(PTHREAD_LIBS) $(shell getconf LFS_CFLAGS) +CFLAGS = $(WARNINGS) -g $(shell getconf LFS_CFLAGS) CFLAGS += -D__EXPORTED_HEADERS__ -I../../include/uapi -I../../include -- 1.7.4.1 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH V3 13/14] Drivers: hv: vmbus: On write cleanup the logic to interrupt the host
From: K. Y. Srinivasan Signal the host when we determine the host is to be signaled. The currrent code determines the need to signal in the ringbuffer code and actually issues the signal elsewhere. This can result in the host viewing this interrupt as spurious since the host may also poll the channel. Make the necessary adjustments. Signed-off-by: K. Y. Srinivasan --- V2: Export vmbus_setevent() symbol; build issue reported by Greg drivers/hv/channel.c | 99 + drivers/hv/hyperv_vmbus.h |6 +- drivers/hv/ring_buffer.c | 30 + include/linux/hyperv.h|1 + 4 files changed, 35 insertions(+), 101 deletions(-) diff --git a/drivers/hv/channel.c b/drivers/hv/channel.c index 5e482d7..8a8148f 100644 --- a/drivers/hv/channel.c +++ b/drivers/hv/channel.c @@ -39,7 +39,7 @@ * vmbus_setevent- Trigger an event notification on the specified * channel. */ -static void vmbus_setevent(struct vmbus_channel *channel) +void vmbus_setevent(struct vmbus_channel *channel) { struct hv_monitor_page *monitorpage; @@ -65,6 +65,7 @@ static void vmbus_setevent(struct vmbus_channel *channel) vmbus_set_event(channel); } } +EXPORT_SYMBOL_GPL(vmbus_setevent); /* * vmbus_open - Open the specified channel. @@ -635,8 +636,6 @@ int vmbus_sendpacket_ctl(struct vmbus_channel *channel, void *buffer, u32 packetlen_aligned = ALIGN(packetlen, sizeof(u64)); struct kvec bufferlist[3]; u64 aligned_data = 0; - int ret; - bool signal = false; bool lock = channel->acquire_ring_lock; int num_vecs = ((bufferlen != 0) ? 3 : 1); @@ -656,41 +655,9 @@ int vmbus_sendpacket_ctl(struct vmbus_channel *channel, void *buffer, bufferlist[2].iov_base = &aligned_data; bufferlist[2].iov_len = (packetlen_aligned - packetlen); - ret = hv_ringbuffer_write(&channel->outbound, bufferlist, num_vecs, - &signal, lock, channel->signal_policy); - - /* -* Signalling the host is conditional on many factors: -* 1. The ring state changed from being empty to non-empty. -*This is tracked by the variable "signal". -* 2. The variable kick_q tracks if more data will be placed -*on the ring. We will not signal if more data is -*to be placed. -* -* Based on the channel signal state, we will decide -* which signaling policy will be applied. -* -* If we cannot write to the ring-buffer; signal the host -* even if we may not have written anything. This is a rare -* enough condition that it should not matter. -* NOTE: in this case, the hvsock channel is an exception, because -* it looks the host side's hvsock implementation has a throttling -* mechanism which can hurt the performance otherwise. -* -* KYS: Oct. 30, 2016: -* It looks like Windows hosts have logic to deal with DOS attacks that -* can be triggered if it receives interrupts when it is not expecting -* the interrupt. The host expects interrupts only when the ring -* transitions from empty to non-empty (or full to non full on the guest -* to host ring). -* So, base the signaling decision solely on the ring state until the -* host logic is fixed. -*/ - - if (((ret == 0) && signal)) - vmbus_setevent(channel); + return hv_ringbuffer_write(channel, bufferlist, num_vecs, + lock, kick_q); - return ret; } EXPORT_SYMBOL(vmbus_sendpacket_ctl); @@ -731,7 +698,6 @@ int vmbus_sendpacket_pagebuffer_ctl(struct vmbus_channel *channel, u32 flags, bool kick_q) { - int ret; int i; struct vmbus_channel_packet_page_buffer desc; u32 descsize; @@ -739,7 +705,6 @@ int vmbus_sendpacket_pagebuffer_ctl(struct vmbus_channel *channel, u32 packetlen_aligned; struct kvec bufferlist[3]; u64 aligned_data = 0; - bool signal = false; bool lock = channel->acquire_ring_lock; if (pagecount > MAX_PAGE_BUFFER_COUNT) @@ -777,38 +742,8 @@ int vmbus_sendpacket_pagebuffer_ctl(struct vmbus_channel *channel, bufferlist[2].iov_base = &aligned_data; bufferlist[2].iov_len = (packetlen_aligned - packetlen); - ret = hv_ringbuffer_write(&channel->outbound, bufferlist, 3, - &signal, lock, channel->signal_policy); - - /* -* Signalling the host is conditional on many factors: -* 1. The ring state changed from being empty to non-empty. -*This is tracked by the variable "signal". -* 2. The variable kick_q tracks if more data will be placed -*on the ring. We will not signal if more data is -
[PATCH V3 01/14] Drivers: hv: ring_buffer: count on wrap around mappings in get_next_pkt_raw() (v2)
From: Vitaly Kuznetsov With wrap around mappings in place we can always provide drivers with direct links to packets on the ring buffer, even when they wrap around. Do the required updates to get_next_pkt_raw()/put_pkt_raw() The first version of this commit was reverted (65a532f3d50a) to deal with cross-tree merge issues which are (hopefully) resolved now. Signed-off-by: Vitaly Kuznetsov Signed-off-by: K. Y. Srinivasan Tested-by: Dexuan Cui --- include/linux/hyperv.h | 32 +++- 1 files changed, 11 insertions(+), 21 deletions(-) diff --git a/include/linux/hyperv.h b/include/linux/hyperv.h index 6824556..42ae6a5 100644 --- a/include/linux/hyperv.h +++ b/include/linux/hyperv.h @@ -1526,31 +1526,23 @@ static inline bool hv_need_to_signal_on_read(struct hv_ring_buffer_info *rbi) get_next_pkt_raw(struct vmbus_channel *channel) { struct hv_ring_buffer_info *ring_info = &channel->inbound; - u32 read_loc = ring_info->priv_read_index; + u32 priv_read_loc = ring_info->priv_read_index; void *ring_buffer = hv_get_ring_buffer(ring_info); - struct vmpacket_descriptor *cur_desc; - u32 packetlen; u32 dsize = ring_info->ring_datasize; - u32 delta = read_loc - ring_info->ring_buffer->read_index; + /* +* delta is the difference between what is available to read and +* what was already consumed in place. We commit read index after +* the whole batch is processed. +*/ + u32 delta = priv_read_loc >= ring_info->ring_buffer->read_index ? + priv_read_loc - ring_info->ring_buffer->read_index : + (dsize - ring_info->ring_buffer->read_index) + priv_read_loc; u32 bytes_avail_toread = (hv_get_bytes_to_read(ring_info) - delta); if (bytes_avail_toread < sizeof(struct vmpacket_descriptor)) return NULL; - if ((read_loc + sizeof(*cur_desc)) > dsize) - return NULL; - - cur_desc = ring_buffer + read_loc; - packetlen = cur_desc->len8 << 3; - - /* -* If the packet under consideration is wrapping around, -* return failure. -*/ - if ((read_loc + packetlen + VMBUS_PKT_TRAILER) > (dsize - 1)) - return NULL; - - return cur_desc; + return ring_buffer + priv_read_loc; } /* @@ -1562,16 +1554,14 @@ static inline void put_pkt_raw(struct vmbus_channel *channel, struct vmpacket_descriptor *desc) { struct hv_ring_buffer_info *ring_info = &channel->inbound; - u32 read_loc = ring_info->priv_read_index; u32 packetlen = desc->len8 << 3; u32 dsize = ring_info->ring_datasize; - if ((read_loc + packetlen + VMBUS_PKT_TRAILER) > dsize) - BUG(); /* * Include the packet trailer. */ ring_info->priv_read_index += packetlen + VMBUS_PKT_TRAILER; + ring_info->priv_read_index %= dsize; } /* -- 1.7.4.1 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH V3 03/14] Drivers: hv: utils: Fix the mapping between host version and protocol to use
From: Alex Ng We should intentionally declare the protocols to use for every known host and default to using the latest protocol if the host is unknown or new. Signed-off-by: Alex Ng Signed-off-by: K. Y. Srinivasan --- V3: Addressed Stephen's comment drivers/hv/hv_util.c |9 ++--- 1 files changed, 6 insertions(+), 3 deletions(-) diff --git a/drivers/hv/hv_util.c b/drivers/hv/hv_util.c index bcd0630..e770774 100644 --- a/drivers/hv/hv_util.c +++ b/drivers/hv/hv_util.c @@ -389,16 +389,19 @@ static int util_probe(struct hv_device *dev, ts_srv_version = TS_VERSION_1; hb_srv_version = HB_VERSION_1; break; - case(VERSION_WIN10): + case VERSION_WIN7: + case VERSION_WIN8: + case VERSION_WIN8_1: util_fw_version = UTIL_FW_VERSION; sd_srv_version = SD_VERSION; - ts_srv_version = TS_VERSION; + ts_srv_version = TS_VERSION_3; hb_srv_version = HB_VERSION; break; + case VERSION_WIN10: default: util_fw_version = UTIL_FW_VERSION; sd_srv_version = SD_VERSION; - ts_srv_version = TS_VERSION_3; + ts_srv_version = TS_VERSION; hb_srv_version = HB_VERSION; } -- 1.7.4.1 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH V3 14/14] Drivers: hv: vmbus: On the read path cleanup the logic to interrupt the host
From: K. Y. Srinivasan Signal the host when we determine the host is to be signaled - on th read path. The currrent code determines the need to signal in the ringbuffer code and actually issues the signal elsewhere. This can result in the host viewing this interrupt as spurious since the host may also poll the channel. Make the necessary adjustments. Signed-off-by: K. Y. Srinivasan --- drivers/hv/channel.c | 11 ++- drivers/hv/hyperv_vmbus.h |4 ++-- drivers/hv/ring_buffer.c |7 --- include/linux/hyperv.h| 12 ++-- 4 files changed, 14 insertions(+), 20 deletions(-) diff --git a/drivers/hv/channel.c b/drivers/hv/channel.c index 8a8148f..5fb4c6d 100644 --- a/drivers/hv/channel.c +++ b/drivers/hv/channel.c @@ -879,16 +879,9 @@ int vmbus_sendpacket_multipagebuffer(struct vmbus_channel *channel, u32 bufferlen, u32 *buffer_actual_len, u64 *requestid, bool raw) { - int ret; - bool signal = false; - - ret = hv_ringbuffer_read(&channel->inbound, buffer, bufferlen, -buffer_actual_len, requestid, &signal, raw); + return hv_ringbuffer_read(channel, buffer, bufferlen, + buffer_actual_len, requestid, raw); - if (signal) - vmbus_setevent(channel); - - return ret; } int vmbus_recvpacket(struct vmbus_channel *channel, void *buffer, diff --git a/drivers/hv/hyperv_vmbus.h b/drivers/hv/hyperv_vmbus.h index 2d42ebe..0675b39 100644 --- a/drivers/hv/hyperv_vmbus.h +++ b/drivers/hv/hyperv_vmbus.h @@ -532,9 +532,9 @@ int hv_ringbuffer_write(struct vmbus_channel *channel, u32 kv_count, bool lock, bool kick_q); -int hv_ringbuffer_read(struct hv_ring_buffer_info *inring_info, +int hv_ringbuffer_read(struct vmbus_channel *channel, void *buffer, u32 buflen, u32 *buffer_actual_len, - u64 *requestid, bool *signal, bool raw); + u64 *requestid, bool raw); void hv_ringbuffer_get_debuginfo(struct hv_ring_buffer_info *ring_info, struct hv_ring_buffer_debug_info *debug_info); diff --git a/drivers/hv/ring_buffer.c b/drivers/hv/ring_buffer.c index 4af7130..cd49cb1 100644 --- a/drivers/hv/ring_buffer.c +++ b/drivers/hv/ring_buffer.c @@ -353,9 +353,9 @@ int hv_ringbuffer_write(struct vmbus_channel *channel, return 0; } -int hv_ringbuffer_read(struct hv_ring_buffer_info *inring_info, +int hv_ringbuffer_read(struct vmbus_channel *channel, void *buffer, u32 buflen, u32 *buffer_actual_len, - u64 *requestid, bool *signal, bool raw) + u64 *requestid, bool raw) { u32 bytes_avail_toread; u32 next_read_location = 0; @@ -364,6 +364,7 @@ int hv_ringbuffer_read(struct hv_ring_buffer_info *inring_info, u32 offset; u32 packetlen; int ret = 0; + struct hv_ring_buffer_info *inring_info = &channel->inbound; if (buflen <= 0) return -EINVAL; @@ -421,7 +422,7 @@ int hv_ringbuffer_read(struct hv_ring_buffer_info *inring_info, /* Update the read index */ hv_set_next_read_location(inring_info, next_read_location); - *signal = hv_need_to_signal_on_read(inring_info); + hv_signal_on_read(channel); return ret; } diff --git a/include/linux/hyperv.h b/include/linux/hyperv.h index 8cf78ed..fdb0a87 100644 --- a/include/linux/hyperv.h +++ b/include/linux/hyperv.h @@ -1487,10 +1487,11 @@ int vmbus_send_tl_connect_request(const uuid_le *shv_guest_servie_id, *there is room for the producer to send the pending packet. */ -static inline bool hv_need_to_signal_on_read(struct hv_ring_buffer_info *rbi) +static inline void hv_signal_on_read(struct vmbus_channel *channel) { u32 cur_write_sz; u32 pending_sz; + struct hv_ring_buffer_info *rbi = &channel->inbound; /* * Issue a full memory barrier before making the signaling decision. @@ -1508,14 +1509,14 @@ static inline bool hv_need_to_signal_on_read(struct hv_ring_buffer_info *rbi) pending_sz = READ_ONCE(rbi->ring_buffer->pending_send_sz); /* If the other end is not blocked on write don't bother. */ if (pending_sz == 0) - return false; + return; cur_write_sz = hv_get_bytes_to_write(rbi); if (cur_write_sz >= pending_sz) - return true; + vmbus_setevent(channel); - return false; + return; } /* @@ -1587,8 +1588,7 @@ static inline void commit_rd_index(struct vmbus_channel *channel) virt_rmb(); ring_info->ring_buffer->read_index = ring_info->priv_read_index; - if (hv_need_to_signal_on_read(ring_info)) - vmbus_set_event(channel); + hv_signal_on_read(channel); } -- 1.7.4.1
[PATCH V3 07/14] Drivers: hv: vss: Operation timeouts should match host expectation
From: Alex Ng Increase the timeout of backup operations. When system is under I/O load, it needs more time to freeze. These timeout values should also match the host timeout values more closely. Signed-off-by: Alex Ng Signed-off-by: K. Y. Srinivasan --- drivers/hv/hv_snapshot.c |8 ++-- 1 files changed, 6 insertions(+), 2 deletions(-) diff --git a/drivers/hv/hv_snapshot.c b/drivers/hv/hv_snapshot.c index 5c95ba1..eee238c 100644 --- a/drivers/hv/hv_snapshot.c +++ b/drivers/hv/hv_snapshot.c @@ -31,7 +31,10 @@ #define VSS_MINOR 0 #define VSS_VERSION(VSS_MAJOR << 16 | VSS_MINOR) -#define VSS_USERSPACE_TIMEOUT (msecs_to_jiffies(10 * 1000)) +/* + * Timeout values are based on expecations from host + */ +#define VSS_FREEZE_TIMEOUT (15 * 60) /* * Global state maintained for transaction that is being processed. For a class @@ -186,7 +189,8 @@ static void vss_send_op(void) vss_transaction.state = HVUTIL_USERSPACE_REQ; - schedule_delayed_work(&vss_timeout_work, VSS_USERSPACE_TIMEOUT); + schedule_delayed_work(&vss_timeout_work, op == VSS_OP_FREEZE ? + VSS_FREEZE_TIMEOUT * HZ : HV_UTIL_TIMEOUT * HZ); rc = hvutil_transport_send(hvt, vss_msg, sizeof(*vss_msg), NULL); if (rc) { -- 1.7.4.1 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH V3 04/14] Drivers: hv: balloon: Disable hot add when CONFIG_MEMORY_HOTPLUG is not set
From: Alex Ng If the guest does not support memory hotplugging, it should respond to the host with zero pages added and successful result code. This signals to the host that hotplugging is not supported and the host will avoid sending future hot-add requests. Signed-off-by: Alex Ng Signed-off-by: K. Y. Srinivasan --- drivers/hv/hv_balloon.c |4 1 files changed, 4 insertions(+), 0 deletions(-) diff --git a/drivers/hv/hv_balloon.c b/drivers/hv/hv_balloon.c index fdf8da9..8f932aa 100644 --- a/drivers/hv/hv_balloon.c +++ b/drivers/hv/hv_balloon.c @@ -1501,7 +1501,11 @@ static int balloon_probe(struct hv_device *dev, struct dm_version_request version_req; struct dm_capabilities cap_msg; +#ifdef CONFIG_MEMORY_HOTPLUG do_hot_add = hot_add; +#else + do_hot_add = false; +#endif /* * First allocate a send buffer. -- 1.7.4.1 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH V3 10/14] tools: hv: fix a compile warning in snprintf
From: Weibing Zhang hv_kvp_daemon.c: In function .kvp_mac_to_if_name.: hv_kvp_daemon.c:705:2: warning: format not a string literal and no format arguments [-Wformat-security] snprintf(dev_id, sizeof(dev_id), kvp_net_dir); ^ hv_kvp_daemon.c:705:2: warning: format not a string literal and no format arguments [-Wformat-security] Signed-off-by: Weibing Zhang Signed-off-by: K. Y. Srinivasan --- tools/hv/hv_kvp_daemon.c |2 +- 1 files changed, 1 insertions(+), 1 deletions(-) diff --git a/tools/hv/hv_kvp_daemon.c b/tools/hv/hv_kvp_daemon.c index bc7adb8..bb0719b 100644 --- a/tools/hv/hv_kvp_daemon.c +++ b/tools/hv/hv_kvp_daemon.c @@ -702,7 +702,7 @@ void kvp_get_os_info(void) if (dir == NULL) return NULL; - snprintf(dev_id, sizeof(dev_id), kvp_net_dir); + snprintf(dev_id, sizeof(dev_id), "%s", kvp_net_dir); q = dev_id + strlen(kvp_net_dir); while ((entry = readdir(dir)) != NULL) { -- 1.7.4.1 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH V3 05/14] Drivers: hv: balloon: Add logging for dynamic memory operations
From: Alex Ng Added logging to help troubleshoot common ballooning, hot add, and versioning issues. Signed-off-by: Alex Ng Signed-off-by: K. Y. Srinivasan --- drivers/hv/hv_balloon.c | 31 --- 1 files changed, 28 insertions(+), 3 deletions(-) diff --git a/drivers/hv/hv_balloon.c b/drivers/hv/hv_balloon.c index 8f932aa..8cac29a 100644 --- a/drivers/hv/hv_balloon.c +++ b/drivers/hv/hv_balloon.c @@ -564,6 +564,11 @@ struct hv_dynmem_device { * next version to try. */ __u32 next_version; + + /* +* The negotiated version agreed by host. +*/ + __u32 version; }; static struct hv_dynmem_device dm_device; @@ -645,6 +650,7 @@ static void hv_bring_pgs_online(struct hv_hotadd_state *has, { int i; + pr_debug("Online %lu pages starting at pfn 0x%lx\n", size, start_pfn); for (i = 0; i < size; i++) hv_page_online_one(has, pfn_to_page(start_pfn + i)); } @@ -685,7 +691,7 @@ static void hv_mem_hot_add(unsigned long start, unsigned long size, (HA_CHUNK << PAGE_SHIFT)); if (ret) { - pr_info("hot_add memory failed error is %d\n", ret); + pr_warn("hot_add memory failed error is %d\n", ret); if (ret == -EEXIST) { /* * This error indicates that the error @@ -814,6 +820,9 @@ static unsigned long handle_pg_range(unsigned long pg_start, unsigned long old_covered_state; unsigned long res = 0, flags; + pr_debug("Hot adding %lu pages starting at pfn 0x%lx.\n", pg_count, + pg_start); + spin_lock_irqsave(&dm_device.ha_lock, flags); list_for_each_entry(has, &dm_device.ha_region_list, list) { /* @@ -1196,8 +1205,6 @@ static unsigned int alloc_balloon_pages(struct hv_dynmem_device *dm, return num_pages; } - - static void balloon_up(struct work_struct *dummy) { unsigned int num_pages = dm_device.balloon_wrk.num_pages; @@ -1224,6 +1231,10 @@ static void balloon_up(struct work_struct *dummy) /* Refuse to balloon below the floor, keep the 2M granularity. */ if (avail_pages < num_pages || avail_pages - num_pages < floor) { + pr_warn("Balloon request will be partially fulfilled. %s\n", + avail_pages < num_pages ? "Not enough memory." : + "Balloon floor reached."); + num_pages = avail_pages > floor ? (avail_pages - floor) : 0; num_pages -= num_pages % PAGES_IN_2M; } @@ -1245,6 +1256,9 @@ static void balloon_up(struct work_struct *dummy) } if (num_ballooned == 0 || num_ballooned == num_pages) { + pr_debug("Ballooned %u out of %u requested pages.\n", + num_pages, dm_device.balloon_wrk.num_pages); + bl_resp->more_pages = 0; done = true; dm_device.state = DM_INITIALIZED; @@ -1292,12 +1306,16 @@ static void balloon_down(struct hv_dynmem_device *dm, int range_count = req->range_count; struct dm_unballoon_response resp; int i; + unsigned int prev_pages_ballooned = dm->num_pages_ballooned; for (i = 0; i < range_count; i++) { free_balloon_pages(dm, &range_array[i]); complete(&dm_device.config_event); } + pr_debug("Freed %u ballooned pages.\n", + prev_pages_ballooned - dm->num_pages_ballooned); + if (req->more_pages == 1) return; @@ -1365,6 +1383,7 @@ static void version_resp(struct hv_dynmem_device *dm, version_req.hdr.size = sizeof(struct dm_version_request); version_req.hdr.trans_id = atomic_inc_return(&trans_id); version_req.version.version = dm->next_version; + dm->version = version_req.version.version; /* * Set the next version to try in case current version fails. @@ -1557,6 +1576,7 @@ static int balloon_probe(struct hv_device *dev, version_req.hdr.trans_id = atomic_inc_return(&trans_id); version_req.version.version = DYNMEM_PROTOCOL_VERSION_WIN10; version_req.is_last_attempt = 0; + dm_device.version = version_req.version.version; ret = vmbus_sendpacket(dev->channel, &version_req, sizeof(struct dm_version_request), @@ -1579,6 +1599,11 @@ static int balloon_probe(struct hv_device *dev, ret = -ETIMEDOUT; goto probe_error2; } + + pr_info("Using Dynamic Memory protocol version %u.%u\n", + DYNMEM_MAJOR_VERSION(dm_device.version), + DYNMEM_MINOR_VERSION(dm_device.version)); + /* * Now submit our capabilities to the host. */ -- 1.7.4.1
[PATCH V3 06/14] Drivers: hv: vss: Improve log messages.
From: Alex Ng Adding log messages to help troubleshoot error cases and transaction handling. Signed-off-by: Alex Ng Signed-off-by: K. Y. Srinivasan --- drivers/hv/hv_snapshot.c | 25 +++-- 1 files changed, 19 insertions(+), 6 deletions(-) diff --git a/drivers/hv/hv_snapshot.c b/drivers/hv/hv_snapshot.c index a670713..5c95ba1 100644 --- a/drivers/hv/hv_snapshot.c +++ b/drivers/hv/hv_snapshot.c @@ -120,7 +120,7 @@ static int vss_handle_handshake(struct hv_vss_msg *vss_msg) default: return -EINVAL; } - pr_debug("VSS: userspace daemon ver. %d connected\n", dm_reg_value); + pr_info("VSS: userspace daemon ver. %d connected\n", dm_reg_value); return 0; } @@ -128,8 +128,10 @@ static int vss_on_msg(void *msg, int len) { struct hv_vss_msg *vss_msg = (struct hv_vss_msg *)msg; - if (len != sizeof(*vss_msg)) + if (len != sizeof(*vss_msg)) { + pr_debug("VSS: Message size does not match length\n"); return -EINVAL; + } if (vss_msg->vss_hdr.operation == VSS_OP_REGISTER || vss_msg->vss_hdr.operation == VSS_OP_REGISTER1) { @@ -137,8 +139,11 @@ static int vss_on_msg(void *msg, int len) * Don't process registration messages if we're in the middle * of a transaction processing. */ - if (vss_transaction.state > HVUTIL_READY) + if (vss_transaction.state > HVUTIL_READY) { + pr_debug("VSS: Got unexpected registration request\n"); return -EINVAL; + } + return vss_handle_handshake(vss_msg); } else if (vss_transaction.state == HVUTIL_USERSPACE_REQ) { vss_transaction.state = HVUTIL_USERSPACE_RECV; @@ -155,7 +160,7 @@ static int vss_on_msg(void *msg, int len) } } else { /* This is a spurious call! */ - pr_warn("VSS: Transaction not active\n"); + pr_debug("VSS: Transaction not active\n"); return -EINVAL; } return 0; @@ -168,8 +173,10 @@ static void vss_send_op(void) struct hv_vss_msg *vss_msg; /* The transaction state is wrong. */ - if (vss_transaction.state != HVUTIL_HOSTMSG_RECEIVED) + if (vss_transaction.state != HVUTIL_HOSTMSG_RECEIVED) { + pr_debug("VSS: Unexpected attempt to send to daemon\n"); return; + } vss_msg = kzalloc(sizeof(*vss_msg), GFP_KERNEL); if (!vss_msg) @@ -210,9 +217,13 @@ static void vss_handle_request(struct work_struct *dummy) case VSS_OP_HOT_BACKUP: if (vss_transaction.state < HVUTIL_READY) { /* Userspace is not registered yet */ + pr_debug("VSS: Not ready for request.\n"); vss_respond_to_host(HV_E_FAIL); return; } + + pr_debug("VSS: Received request for op code: %d\n", + vss_transaction.msg->vss_hdr.operation); vss_transaction.state = HVUTIL_HOSTMSG_RECEIVED; vss_send_op(); return; @@ -353,8 +364,10 @@ static void vss_on_reset(void) hvt = hvutil_transport_init(vss_devname, CN_VSS_IDX, CN_VSS_VAL, vss_on_msg, vss_on_reset); - if (!hvt) + if (!hvt) { + pr_warn("VSS: Failed to initialize transport\n"); return -EFAULT; + } return 0; } -- 1.7.4.1 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH V3 08/14] Drivers: hv: balloon: Fix info request to show max page count
From: Alex Ng Balloon driver was only printing the size of the info blob and not the actual content. This fixes it so that the info blob (max page count as configured in Hyper-V) is printed out. Signed-off-by: Alex Ng Signed-off-by: K. Y. Srinivasan --- drivers/hv/hv_balloon.c |9 +++-- 1 files changed, 7 insertions(+), 2 deletions(-) diff --git a/drivers/hv/hv_balloon.c b/drivers/hv/hv_balloon.c index 8cac29a..14c3dc4 100644 --- a/drivers/hv/hv_balloon.c +++ b/drivers/hv/hv_balloon.c @@ -1034,8 +1034,13 @@ static void process_info(struct hv_dynmem_device *dm, struct dm_info_msg *msg) switch (info_hdr->type) { case INFO_TYPE_MAX_PAGE_CNT: - pr_info("Received INFO_TYPE_MAX_PAGE_CNT\n"); - pr_info("Data Size is %d\n", info_hdr->data_size); + if (info_hdr->data_size == sizeof(__u64)) { + __u64 *max_page_count = (__u64 *)&info_hdr[1]; + + pr_info("INFO_TYPE_MAX_PAGE_CNT = %llu\n", + *max_page_count); + } + break; default: pr_info("Received Unknown type: %d\n", info_hdr->type); -- 1.7.4.1 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH] Staging: fsl-mc: Changed allocation types into the types that the kernel prefers
Fixed reports from checkpatch that pointed out that the code was not using the preferred types of the kernel. Signed-off-by: Shiva Kerdel --- drivers/staging/fsl-mc/bus/dpmcp.h | 80 - drivers/staging/fsl-mc/include/mc-bus.h | 4 +- drivers/staging/fsl-mc/include/mc.h | 2 +- 3 files changed, 43 insertions(+), 43 deletions(-) diff --git a/drivers/staging/fsl-mc/bus/dpmcp.h b/drivers/staging/fsl-mc/bus/dpmcp.h index fe79d4d..684c5b5 100644 --- a/drivers/staging/fsl-mc/bus/dpmcp.h +++ b/drivers/staging/fsl-mc/bus/dpmcp.h @@ -39,7 +39,7 @@ struct fsl_mc_io; int dpmcp_open(struct fsl_mc_io *mc_io, - uint32_t cmd_flags, + u32 cmd_flags, int dpmcp_id, uint16_t *token); @@ -47,7 +47,7 @@ int dpmcp_open(struct fsl_mc_io *mc_io, #define DPMCP_GET_PORTAL_ID_FROM_POOL (-1) int dpmcp_close(struct fsl_mc_io *mc_io, - uint32_t cmd_flags, + u32 cmd_flags, uint16_t token); /** @@ -60,16 +60,16 @@ struct dpmcp_cfg { }; int dpmcp_create(struct fsl_mc_io *mc_io, -uint32_t cmd_flags, +u32cmd_flags, const struct dpmcp_cfg *cfg, - uint16_t*token); +uint16_t *token); int dpmcp_destroy(struct fsl_mc_io *mc_io, - uint32_t cmd_flags, + u32 cmd_flags, uint16_t token); int dpmcp_reset(struct fsl_mc_io *mc_io, - uint32_t cmd_flags, + u32 cmd_flags, uint16_t token); /* IRQ */ @@ -85,53 +85,53 @@ int dpmcp_reset(struct fsl_mc_io *mc_io, * @irq_num: A user defined number associated with this IRQ */ struct dpmcp_irq_cfg { -uint64_t paddr; -uint32_t val; +u64paddr; +u32val; intirq_num; }; int dpmcp_set_irq(struct fsl_mc_io *mc_io, - uint32_t cmd_flags, - uint16_t token, -uint8_tirq_index, + u32 cmd_flags, + u16 token, + u8irq_index, struct dpmcp_irq_cfg *irq_cfg); int dpmcp_get_irq(struct fsl_mc_io *mc_io, - uint32_t cmd_flags, - uint16_t token, -uint8_tirq_index, -int*type, -struct dpmcp_irq_cfg *irq_cfg); + u32 cmd_flags, + u16 token, + u8irq_index, + int *type, + struct dpmcp_irq_cfg *irq_cfg); int dpmcp_set_irq_enable(struct fsl_mc_io *mc_io, -uint32_t cmd_flags, -uint16_t token, - uint8_t irq_index, - uint8_t en); +u32cmd_flags, +u16token, +u8 irq_index, +uint8_ten); int dpmcp_get_irq_enable(struct fsl_mc_io *mc_io, -uint32_t cmd_flags, -uint16_t token, - uint8_t irq_index, - uint8_t *en); +u32cmd_flags, +u16token, +u8 irq_index, +uint8_t*en); int dpmcp_set_irq_mask(struct fsl_mc_io*mc_io, - uint32_t cmd_flags, - uint16_t token, - uint8_t irq_index, - uint32_t mask); + u32 cmd_flags, + u16 token, + u8 irq_index, + uint32_t mask); int dpmcp_get_irq_mask(struct fsl_mc_io*mc_io, - uint32_t cmd_flags, - uint16_t token, - uint8_t irq_index, - uint32_t *mask); + u32 cmd_flags, + u16 token, + u8 irq_index, + uint32_t *mask); int dpmcp_get_irq_status(struct fsl_mc_io *mc_io, -uint32_t cmd_flags,
Re: [PATCH 02/14] staging/lustre/ldlm: fix export reference problem
> From: Hongchao Zhang > > 1, in client_import_del_conn, the export returned from >class_conn2export is not released after using it. > > 2, in ptlrpc_connect_interpret, the export is not released >if the connect_flags isn't compatible. Reviewed-by: James Simmons > Signed-off-by: Hongchao Zhang > Reviewed-on: http://review.whamcloud.com/22031 > Intel-bug-id: https://jira.hpdd.intel.com/browse/LU-8500 > Reviewed-by: James Simmons > Reviewed-by: Bobi Jam > Signed-off-by: Oleg Drokin > --- > drivers/staging/lustre/lustre/ldlm/ldlm_lib.c | 3 +++ > drivers/staging/lustre/lustre/ptlrpc/import.c | 19 ++- > 2 files changed, 13 insertions(+), 9 deletions(-) > > diff --git a/drivers/staging/lustre/lustre/ldlm/ldlm_lib.c > b/drivers/staging/lustre/lustre/ldlm/ldlm_lib.c > index 4f9480e..06d3cc6 100644 > --- a/drivers/staging/lustre/lustre/ldlm/ldlm_lib.c > +++ b/drivers/staging/lustre/lustre/ldlm/ldlm_lib.c > @@ -170,6 +170,9 @@ int client_import_del_conn(struct obd_import *imp, struct > obd_uuid *uuid) > ptlrpc_connection_put(dlmexp->exp_connection); > dlmexp->exp_connection = NULL; > } > + > + if (dlmexp) > + class_export_put(dlmexp); > } > > list_del(&imp_conn->oic_item); > diff --git a/drivers/staging/lustre/lustre/ptlrpc/import.c > b/drivers/staging/lustre/lustre/ptlrpc/import.c > index 46ba5a4..05fd92d 100644 > --- a/drivers/staging/lustre/lustre/ptlrpc/import.c > +++ b/drivers/staging/lustre/lustre/ptlrpc/import.c > @@ -972,6 +972,16 @@ static int ptlrpc_connect_interpret(const struct lu_env > *env, > > spin_unlock(&imp->imp_lock); > > + if (!exp) { > + /* This could happen if export is cleaned during the > + * connect attempt > + */ > + CERROR("%s: missing export after connect\n", > +imp->imp_obd->obd_name); > + rc = -ENODEV; > + goto out; > + } > + > /* check that server granted subset of flags we asked for. */ > if ((ocd->ocd_connect_flags & imp->imp_connect_flags_orig) != > ocd->ocd_connect_flags) { > @@ -982,15 +992,6 @@ static int ptlrpc_connect_interpret(const struct lu_env > *env, > goto out; > } > > - if (!exp) { > - /* This could happen if export is cleaned during the > - * connect attempt > - */ > - CERROR("%s: missing export after connect\n", > -imp->imp_obd->obd_name); > - rc = -ENODEV; > - goto out; > - } > old_connect_flags = exp_connect_flags(exp); > exp->exp_connect_data = *ocd; > imp->imp_obd->obd_self_export->exp_connect_data = *ocd; > -- > 2.7.4 > > ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 04/14] staging/lustre/llite: clear inode timestamps after losing UPDATE lock
> From: Niu Yawei > > Otherwise, those leftovers would interfere with new timestamps > especially when the timestamps are set back in time on the other > clients. Reviewed-by: James Simmons > Signed-off-by: Jinshan Xiong > Signed-off-by: Niu Yawei > Reviewed-on: http://review.whamcloud.com/22623 > Intel-bug-id: https://jira.hpdd.intel.com/browse/LU-8446 > Reviewed-by: Bobi Jam > Signed-off-by: Oleg Drokin > --- > drivers/staging/lustre/lustre/llite/namei.c | 10 ++ > 1 file changed, 10 insertions(+) > > diff --git a/drivers/staging/lustre/lustre/llite/namei.c > b/drivers/staging/lustre/lustre/llite/namei.c > index 74d9b73..c268f32 100644 > --- a/drivers/staging/lustre/lustre/llite/namei.c > +++ b/drivers/staging/lustre/lustre/llite/namei.c > @@ -251,6 +251,16 @@ int ll_md_blocking_ast(struct ldlm_lock *lock, struct > ldlm_lock_desc *desc, > PFID(ll_inode2fid(inode)), rc); > } > > + if (bits & MDS_INODELOCK_UPDATE) { > + struct ll_inode_info *lli = ll_i2info(inode); > + > + spin_lock(&lli->lli_lock); > + LTIME_S(inode->i_mtime) = 0; > + LTIME_S(inode->i_atime) = 0; > + LTIME_S(inode->i_ctime) = 0; > + spin_unlock(&lli->lli_lock); > + } > + > if ((bits & MDS_INODELOCK_UPDATE) && S_ISDIR(inode->i_mode)) { > struct ll_inode_info *lli = ll_i2info(inode); > > -- > 2.7.4 > > ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 01/14] staging/lustre/ldlm: Drop unused blocking_refs flock field
> blocking_refs is only used on the server, so drop it on the client. Reviewed-by: James Simmons > Signed-off-by: Oleg Drokin > --- > drivers/staging/lustre/lustre/include/lustre_dlm.h | 2 -- > 1 file changed, 2 deletions(-) > > diff --git a/drivers/staging/lustre/lustre/include/lustre_dlm.h > b/drivers/staging/lustre/lustre/include/lustre_dlm.h > index 1c6b7b8..f770b86 100644 > --- a/drivers/staging/lustre/lustre/include/lustre_dlm.h > +++ b/drivers/staging/lustre/lustre/include/lustre_dlm.h > @@ -550,8 +550,6 @@ struct ldlm_flock { > __u64 owner; > __u64 blocking_owner; > struct obd_export *blocking_export; > - /* Protected by the hash lock */ > - __u32 blocking_refs; > __u32 pid; > }; > > -- > 2.7.4 > > ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Hello...Urgent Pls ..
Dear, I'm Dr. Andrea Thompson, I'm seeking for investment opportunities around the globe and wonder if you can be of assistance. Basically, all I need from you is sincerity, authenticity, integrity protection, virtue, accountability and honor which brings trust in business. As you already know, nothing can undermine a business relationship more completely than lack of trust because trust is the essential precondition upon which all real success depends and the key to trust is action and commitments. Commitments made and commitments honored. Once you agree to this, we shall sign the Equity Investment Agreement where we shall state the profit sharing pattern and all other conditions. kindly reply me to my private email: dr.andrea.thompso...@gmail.com Waiting for your response. Dr. Andrea Thompson . ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 03/14] staging/lustre: conflicting PW & PR extent locks on a client
> From: Andriy Skulysh > > PW lock isn't replayed once a lock is marked > LDLM_FL_CANCELING and glimpse lock doesn't wait for > conflicting locks on the client. So the server will > grant a PR lock in response to the glimpse lock request, > which conflicts with the PW lock in LDLM_FL_CANCELING > state on the client. > > Lock in LDLM_FL_CANCELING state may still have pending IO, > so it should be replayed until LDLM_FL_BL_DONE is set to > avoid granted conflicting lock by a server. Reviewed-by: James Simmons > Seagate-bug-id: MRP-3311 > Signed-off-by: Andriy Skulysh > Reviewed-on: http://review.whamcloud.com/20345 > Intel-bug-id: https://jira.hpdd.intel.com/browse/LU-8175 > Reviewed-by: Jinshan Xiong > Signed-off-by: Oleg Drokin > --- > drivers/staging/lustre/lustre/include/obd_support.h | 3 +++ > drivers/staging/lustre/lustre/ldlm/ldlm_extent.c| 20 > drivers/staging/lustre/lustre/ldlm/ldlm_request.c | 4 ++-- > drivers/staging/lustre/lustre/osc/osc_request.c | 1 + > 4 files changed, 26 insertions(+), 2 deletions(-) > > diff --git a/drivers/staging/lustre/lustre/include/obd_support.h > b/drivers/staging/lustre/lustre/include/obd_support.h > index 7f3f8cd..aaedec7 100644 > --- a/drivers/staging/lustre/lustre/include/obd_support.h > +++ b/drivers/staging/lustre/lustre/include/obd_support.h > @@ -321,6 +321,8 @@ extern char obd_jobid_var[]; > #define OBD_FAIL_LDLM_CP_CB_WAIT4 0x322 > #define OBD_FAIL_LDLM_CP_CB_WAIT5 0x323 > > +#define OBD_FAIL_LDLM_GRANT_CHECK0x32a > + > /* LOCKLESS IO */ > #define OBD_FAIL_LDLM_SET_CONTENTION 0x385 > > @@ -343,6 +345,7 @@ extern char obd_jobid_var[]; > #define OBD_FAIL_OSC_CP_ENQ_RACE 0x410 > #define OBD_FAIL_OSC_NO_GRANT0x411 > #define OBD_FAIL_OSC_DELAY_SETTIME0x412 > +#define OBD_FAIL_OSC_DELAY_IO 0x414 > > #define OBD_FAIL_PTLRPC0x500 > #define OBD_FAIL_PTLRPC_ACK0x501 > diff --git a/drivers/staging/lustre/lustre/ldlm/ldlm_extent.c > b/drivers/staging/lustre/lustre/ldlm/ldlm_extent.c > index ecf472e..a7b34e4 100644 > --- a/drivers/staging/lustre/lustre/ldlm/ldlm_extent.c > +++ b/drivers/staging/lustre/lustre/ldlm/ldlm_extent.c > @@ -193,6 +193,26 @@ void ldlm_extent_add_lock(struct ldlm_resource *res, >* add the locks into grant list, for debug purpose, .. >*/ > ldlm_resource_add_lock(res, &res->lr_granted, lock); > + > + if (OBD_FAIL_CHECK(OBD_FAIL_LDLM_GRANT_CHECK)) { > + struct ldlm_lock *lck; > + > + list_for_each_entry_reverse(lck, &res->lr_granted, > + l_res_link) { > + if (lck == lock) > + continue; > + if (lockmode_compat(lck->l_granted_mode, > + lock->l_granted_mode)) > + continue; > + if (ldlm_extent_overlap(&lck->l_req_extent, > + &lock->l_req_extent)) { > + CDEBUG(D_ERROR, "granting conflicting lock %p > %p\n", > +lck, lock); > + ldlm_resource_dump(D_ERROR, res); > + LBUG(); > + } > + } > + } > } > > /** Remove cancelled lock from resource interval tree. */ > diff --git a/drivers/staging/lustre/lustre/ldlm/ldlm_request.c > b/drivers/staging/lustre/lustre/ldlm/ldlm_request.c > index 43856ff..6e704c7 100644 > --- a/drivers/staging/lustre/lustre/ldlm/ldlm_request.c > +++ b/drivers/staging/lustre/lustre/ldlm/ldlm_request.c > @@ -1846,7 +1846,7 @@ static int ldlm_chain_lock_for_replay(struct ldlm_lock > *lock, void *closure) >* bug 17614: locks being actively cancelled. Get a reference >* on a lock so that it does not disappear under us (e.g. due to cancel) >*/ > - if (!(lock->l_flags & (LDLM_FL_FAILED | LDLM_FL_CANCELING))) { > + if (!(lock->l_flags & (LDLM_FL_FAILED | LDLM_FL_BL_DONE))) { > list_add(&lock->l_pending_chain, list); > LDLM_LOCK_GET(lock); > } > @@ -1915,7 +1915,7 @@ static int replay_one_lock(struct obd_import *imp, > struct ldlm_lock *lock) > int flags; > > /* Bug 11974: Do not replay a lock which is actively being canceled */ > - if (ldlm_is_canceling(lock)) { > + if (ldlm_is_bl_done(lock)) { > LDLM_DEBUG(lock, "Not replaying canceled lock:"); > return 0; > } > diff --git a/drivers/staging/lustre/lustre/osc/osc_request.c > b/drivers/staging/lustre/lustre/osc/osc_request.c > index 091558e..8023561 100644 > --- a/drivers/staging/lustre/lustre/osc/osc_request.c > +++ b/drivers/staging/lustre/lustre/osc/osc_request.c > @@ -1823,6 +1823,7 @@ int osc_build_rpc(const struct lu_env *env, struct > client_obd *cli, > DEBUG_REQ(D_INODE, re
Re: [PATCH 05/14] staging/lustre: Get rid of cl_env hash table
> From: Jinshan Xiong > > cl_env hash table is under heavy contention when there are lots of > processes doing IO at the same time; > reduce lock contention by replacing cl_env cache with percpu array; > remove cl_env_nested_get() and cl_env_nested_put(); > remove cl_env_reenter() and cl_env_reexit(); Reviewed-by: James Simmons > Signed-off-by: Jinshan Xiong > Reviewed-on: http://review.whamcloud.com/20254 > Intel-bug-id: https://jira.hpdd.intel.com/browse/LU-4257 > Reviewed-by: Andreas Dilger > Reviewed-by: Bobi Jam > Signed-off-by: Oleg Drokin > --- > drivers/staging/lustre/lustre/include/cl_object.h | 22 -- > drivers/staging/lustre/lustre/ldlm/ldlm_pool.c | 9 - > drivers/staging/lustre/lustre/llite/file.c | 18 +- > drivers/staging/lustre/lustre/llite/lcommon_cl.c | 11 +- > drivers/staging/lustre/lustre/llite/lcommon_misc.c | 8 +- > drivers/staging/lustre/lustre/llite/llite_mmap.c | 59 ++-- > drivers/staging/lustre/lustre/llite/rw.c | 6 +- > drivers/staging/lustre/lustre/llite/rw26.c | 16 +- > drivers/staging/lustre/lustre/lov/lov_io.c | 5 - > drivers/staging/lustre/lustre/lov/lov_object.c | 7 +- > .../staging/lustre/lustre/obdclass/cl_internal.h | 23 -- > drivers/staging/lustre/lustre/obdclass/cl_io.c | 1 - > drivers/staging/lustre/lustre/obdclass/cl_object.c | 389 > - > drivers/staging/lustre/lustre/obdclass/lu_object.c | 4 - > .../staging/lustre/lustre/obdecho/echo_client.c| 14 +- > drivers/staging/lustre/lustre/osc/osc_cache.c | 6 +- > drivers/staging/lustre/lustre/osc/osc_lock.c | 116 +++--- > drivers/staging/lustre/lustre/osc/osc_page.c | 6 +- > 18 files changed, 183 insertions(+), 537 deletions(-) > > diff --git a/drivers/staging/lustre/lustre/include/cl_object.h > b/drivers/staging/lustre/lustre/include/cl_object.h > index 514d650..3fe26e7 100644 > --- a/drivers/staging/lustre/lustre/include/cl_object.h > +++ b/drivers/staging/lustre/lustre/include/cl_object.h > @@ -2640,35 +2640,13 @@ void cl_sync_io_end(const struct lu_env *env, struct > cl_sync_io *anchor); > * - allocation and destruction of environment is amortized by caching no > * longer used environments instead of destroying them; > * > - * - there is a notion of "current" environment, attached to the kernel > - * data structure representing current thread Top-level lustre code > - * allocates an environment and makes it current, then calls into > - * non-lustre code, that in turn calls lustre back. Low-level lustre > - * code thus called can fetch environment created by the top-level code > - * and reuse it, avoiding additional environment allocation. > - * Right now, three interfaces can attach the cl_env to running thread: > - * - cl_env_get > - * - cl_env_implant > - * - cl_env_reexit(cl_env_reenter had to be called priorly) > - * > * \see lu_env, lu_context, lu_context_key > * @{ > */ > > -struct cl_env_nest { > - int cen_refcheck; > - void *cen_cookie; > -}; > - > struct lu_env *cl_env_get(int *refcheck); > struct lu_env *cl_env_alloc(int *refcheck, __u32 tags); > -struct lu_env *cl_env_nested_get(struct cl_env_nest *nest); > void cl_env_put(struct lu_env *env, int *refcheck); > -void cl_env_nested_put(struct cl_env_nest *nest, struct lu_env *env); > -void *cl_env_reenter(void); > -void cl_env_reexit(void *cookie); > -void cl_env_implant(struct lu_env *env, int *refcheck); > -void cl_env_unplant(struct lu_env *env, int *refcheck); > unsigned int cl_env_cache_purge(unsigned int nr); > struct lu_env *cl_env_percpu_get(void); > void cl_env_percpu_put(struct lu_env *env); > diff --git a/drivers/staging/lustre/lustre/ldlm/ldlm_pool.c > b/drivers/staging/lustre/lustre/ldlm/ldlm_pool.c > index b29c9561..19831c5 100644 > --- a/drivers/staging/lustre/lustre/ldlm/ldlm_pool.c > +++ b/drivers/staging/lustre/lustre/ldlm/ldlm_pool.c > @@ -794,7 +794,6 @@ static unsigned long ldlm_pools_count(ldlm_side_t client, > gfp_t gfp_mask) > int nr_ns; > struct ldlm_namespace *ns; > struct ldlm_namespace *ns_old = NULL; /* loop detection */ > - void *cookie; > > if (client == LDLM_NAMESPACE_CLIENT && !(gfp_mask & __GFP_FS)) > return 0; > @@ -802,8 +801,6 @@ static unsigned long ldlm_pools_count(ldlm_side_t client, > gfp_t gfp_mask) > CDEBUG(D_DLMTRACE, "Request to count %s locks from all pools\n", > client == LDLM_NAMESPACE_CLIENT ? "client" : "server"); > > - cookie = cl_env_reenter(); > - > /* >* Find out how many resources we may release. >*/ > @@ -812,7 +809,6 @@ static unsigned long ldlm_pools_count(ldlm_side_t client, > gfp_t gfp_mask) > mutex_lock(ldlm_namespace_lock(client)); > if (list_empty(ldlm_namespace_list(client))) { > mutex_unlock(ldlm_namespace_lock(client)); > -
Re: [PATCH 07/14] staging/lustre/ldlm: Reinstate ldlm_enqueue_pack()
> The function becomes used again with the next patch, so bring it back > from dead, only this time make it static. Reviewed-by: James Simmons > Reverts: bf2a033360f7 ("staging/lustre/ldlm: Remove unused > ldlm_enqueue_pack()") > Signed-off-by: Oleg Drokin > --- > drivers/staging/lustre/lustre/ldlm/ldlm_request.c | 21 + > 1 file changed, 21 insertions(+) > > diff --git a/drivers/staging/lustre/lustre/ldlm/ldlm_request.c > b/drivers/staging/lustre/lustre/ldlm/ldlm_request.c > index 6e704c7..1b9ae77 100644 > --- a/drivers/staging/lustre/lustre/ldlm/ldlm_request.c > +++ b/drivers/staging/lustre/lustre/ldlm/ldlm_request.c > @@ -657,6 +657,27 @@ int ldlm_prep_enqueue_req(struct obd_export *exp, struct > ptlrpc_request *req, > } > EXPORT_SYMBOL(ldlm_prep_enqueue_req); > > +static struct ptlrpc_request *ldlm_enqueue_pack(struct obd_export *exp, > + int lvb_len) > +{ > + struct ptlrpc_request *req; > + int rc; > + > + req = ptlrpc_request_alloc(class_exp2cliimp(exp), &RQF_LDLM_ENQUEUE); > + if (!req) > + return ERR_PTR(-ENOMEM); > + > + rc = ldlm_prep_enqueue_req(exp, req, NULL, 0); > + if (rc) { > + ptlrpc_request_free(req); > + return ERR_PTR(rc); > + } > + > + req_capsule_set_size(&req->rq_pill, &RMF_DLM_LVB, RCL_SERVER, lvb_len); > + ptlrpc_request_set_replen(req); > + return req; > +} > + > /** > * Client-side lock enqueue. > * > -- > 2.7.4 > > ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 06/14] staging/lustre/llite: drop_caches hangs in cl_inode_fini()
> From: Andrew Perepechko > > This patch releases cl_pages on error in ll_write_begin() > to avoid memory and object reference leaks. Also, it > reuses per-cpu lu_env in ll_invalidatepage() in the same > way as done in ll_releasepage(). Reviewed-by: James Simmons > Signed-off-by: Andrew Perepechko > Seagate-bug-id: MRP-3504 > Reviewed-on: http://review.whamcloud.com/22745 > Intel-bug-id: https://jira.hpdd.intel.com/browse/LU-8509 > Reviewed-by: Jinshan Xiong > Reviewed-by: Bobi Jam > Signed-off-by: Oleg Drokin > --- > drivers/staging/lustre/lustre/llite/rw26.c | 36 > -- > 1 file changed, 19 insertions(+), 17 deletions(-) > > diff --git a/drivers/staging/lustre/lustre/llite/rw26.c > b/drivers/staging/lustre/lustre/llite/rw26.c > index 1a08a9d..ca45b44 100644 > --- a/drivers/staging/lustre/lustre/llite/rw26.c > +++ b/drivers/staging/lustre/lustre/llite/rw26.c > @@ -71,8 +71,6 @@ static void ll_invalidatepage(struct page *vmpage, unsigned > int offset, > struct cl_page *page; > struct cl_object *obj; > > - int refcheck; > - > LASSERT(PageLocked(vmpage)); > LASSERT(!PageWriteback(vmpage)); > > @@ -82,21 +80,21 @@ static void ll_invalidatepage(struct page *vmpage, > unsigned int offset, >* happening with locked page too >*/ > if (offset == 0 && length == PAGE_SIZE) { > - env = cl_env_get(&refcheck); > - if (!IS_ERR(env)) { > - inode = vmpage->mapping->host; > - obj = ll_i2info(inode)->lli_clob; > - if (obj) { > - page = cl_vmpage_page(vmpage, obj); > - if (page) { > - cl_page_delete(env, page); > - cl_page_put(env, page); > - } > - } else { > - LASSERT(vmpage->private == 0); > + /* See the comment in ll_releasepage() */ > + env = cl_env_percpu_get(); > + LASSERT(!IS_ERR(env)); > + inode = vmpage->mapping->host; > + obj = ll_i2info(inode)->lli_clob; > + if (obj) { > + page = cl_vmpage_page(vmpage, obj); > + if (page) { > + cl_page_delete(env, page); > + cl_page_put(env, page); > } > - cl_env_put(env, &refcheck); > + } else { > + LASSERT(vmpage->private == 0); > } > + cl_env_percpu_put(env); > } > } > > @@ -466,9 +464,9 @@ static int ll_write_begin(struct file *file, struct > address_space *mapping, > struct page **pagep, void **fsdata) > { > struct ll_cl_context *lcc; > - const struct lu_env *env; > + const struct lu_env *env = NULL; > struct cl_io *io; > - struct cl_page *page; > + struct cl_page *page = NULL; > struct cl_object *clob = ll_i2info(mapping->host)->lli_clob; > pgoff_t index = pos >> PAGE_SHIFT; > struct page *vmpage = NULL; > @@ -556,6 +554,10 @@ static int ll_write_begin(struct file *file, struct > address_space *mapping, > unlock_page(vmpage); > put_page(vmpage); > } > + if (!IS_ERR_OR_NULL(page)) { > + lu_ref_del(&page->cp_reference, "cl_io", io); > + cl_page_put(env, page); > + } > } else { > *pagep = vmpage; > *fsdata = lcc; > -- > 2.7.4 > > ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 08/14] staging/lustre/ldlm: engage ELC for all ldlm enqueue req
> From: Hongchao Zhang > > If there is no request passed into ldlm_cli_enqueue, the enqueue > request will not engage ELC to drop unneeded locks. currently, > this kind of request is mainly related to EXTENT locks enqueue > requests (except for glimpse EXTENT lock for it has an intent). Reviewed-by: James Simmons > Signed-off-by: Hongchao Zhang > Reviewed-on: http://review.whamcloud.com/21739 > Intel-bug-id: https://jira.hpdd.intel.com/browse/LU-8209 > Reviewed-by: Andreas Dilger > Reviewed-by: Vitaly Fertman > Signed-off-by: Oleg Drokin > --- > drivers/staging/lustre/lustre/ldlm/ldlm_request.c | 21 - > 1 file changed, 4 insertions(+), 17 deletions(-) > > diff --git a/drivers/staging/lustre/lustre/ldlm/ldlm_request.c > b/drivers/staging/lustre/lustre/ldlm/ldlm_request.c > index 1b9ae77..c5d00d1 100644 > --- a/drivers/staging/lustre/lustre/ldlm/ldlm_request.c > +++ b/drivers/staging/lustre/lustre/ldlm/ldlm_request.c > @@ -748,17 +748,14 @@ int ldlm_cli_enqueue(struct obd_export *exp, struct > ptlrpc_request **reqp, > lock->l_last_activity = ktime_get_real_seconds(); > > /* lock not sent to server yet */ > - > if (!reqp || !*reqp) { > - req = ptlrpc_request_alloc_pack(class_exp2cliimp(exp), > - &RQF_LDLM_ENQUEUE, > - LUSTRE_DLM_VERSION, > - LDLM_ENQUEUE); > - if (!req) { > + req = ldlm_enqueue_pack(exp, lvb_len); > + if (IS_ERR(req)) { > failed_lock_cleanup(ns, lock, einfo->ei_mode); > LDLM_LOCK_RELEASE(lock); > - return -ENOMEM; > + return PTR_ERR(req); > } > + > req_passed_in = 0; > if (reqp) > *reqp = req; > @@ -778,16 +775,6 @@ int ldlm_cli_enqueue(struct obd_export *exp, struct > ptlrpc_request **reqp, > body->lock_flags = ldlm_flags_to_wire(*flags); > body->lock_handle[0] = *lockh; > > - /* Continue as normal. */ > - if (!req_passed_in) { > - if (lvb_len > 0) > - req_capsule_extend(&req->rq_pill, > -&RQF_LDLM_ENQUEUE_LVB); > - req_capsule_set_size(&req->rq_pill, &RMF_DLM_LVB, RCL_SERVER, > - lvb_len); > - ptlrpc_request_set_replen(req); > - } > - > /* >* Liblustre client doesn't get extent locks, except for O_APPEND case >* where [0, OBD_OBJECT_EOF] lock is taken, or truncate, where > -- > 2.7.4 > > ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 10/14] staging/lustre/llite: protect from accessing NULL lli_clob
> From: Bobi Jam > > Need to check file's lli_clob object before calling > lov_read_and_clear_async_rc(). Reviewed-by: James Simmons > Signed-off-by: Bobi Jam > Reviewed-by: Jinshan Xiong > Reviewed-by: Oleg Drokin > Reviewed-on: http://review.whamcloud.com/23031 > Intel-bug-id: https://jira.hpdd.intel.com/browse/LU-8682 > Signed-off-by: Oleg Drokin > --- > drivers/staging/lustre/lustre/llite/file.c | 8 +--- > 1 file changed, 5 insertions(+), 3 deletions(-) > > diff --git a/drivers/staging/lustre/lustre/llite/file.c > b/drivers/staging/lustre/lustre/llite/file.c > index c1c7551..7886840 100644 > --- a/drivers/staging/lustre/lustre/llite/file.c > +++ b/drivers/staging/lustre/lustre/llite/file.c > @@ -2328,9 +2328,11 @@ int ll_fsync(struct file *file, loff_t start, loff_t > end, int datasync) > lli->lli_async_rc = 0; > if (rc == 0) > rc = err; > - err = lov_read_and_clear_async_rc(lli->lli_clob); > - if (rc == 0) > - rc = err; > + if (lli->lli_clob) { > + err = lov_read_and_clear_async_rc(lli->lli_clob); > + if (rc == 0) > + rc = err; > + } > } > > err = md_sync(ll_i2sbi(inode)->ll_md_exp, ll_inode2fid(inode), &req); > -- > 2.7.4 > > ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 14/14] staging/lustre: Get rid of LIBLUSTRE_CLIENT and its users
> This define only made sense in a userspace library client, not in the kernel. Reviewed-by: James Simmons > Signed-off-by: Oleg Drokin > --- > drivers/staging/lustre/lustre/include/lustre_lib.h | 2 -- > drivers/staging/lustre/lustre/ldlm/ldlm_request.c | 15 +-- > 2 files changed, 1 insertion(+), 16 deletions(-) > > diff --git a/drivers/staging/lustre/lustre/include/lustre_lib.h > b/drivers/staging/lustre/lustre/include/lustre_lib.h > index 6b23191..27f3148 100644 > --- a/drivers/staging/lustre/lustre/include/lustre_lib.h > +++ b/drivers/staging/lustre/lustre/include/lustre_lib.h > @@ -350,8 +350,6 @@ do { >\ > l_wait_event_exclusive_head(wq, condition, &lwi); \ > }) > > -#define LIBLUSTRE_CLIENT (0) > - > /** @} lib */ > > #endif /* _LUSTRE_LIB_H */ > diff --git a/drivers/staging/lustre/lustre/ldlm/ldlm_request.c > b/drivers/staging/lustre/lustre/ldlm/ldlm_request.c > index c5d00d1..6a96f2c 100644 > --- a/drivers/staging/lustre/lustre/ldlm/ldlm_request.c > +++ b/drivers/staging/lustre/lustre/ldlm/ldlm_request.c > @@ -475,12 +475,7 @@ int ldlm_cli_enqueue_fini(struct obd_export *exp, struct > ptlrpc_request *req, > "client-side enqueue, new policy data"); > } > > - if ((*flags) & LDLM_FL_AST_SENT || > - /* Cancel extent locks as soon as possible on a liblustre client, > - * because it cannot handle asynchronous ASTs robustly (see > - * bug 7311). > - */ > - (LIBLUSTRE_CLIENT && type == LDLM_EXTENT)) { > + if ((*flags) & LDLM_FL_AST_SENT) { > lock_res_and_lock(lock); > lock->l_flags |= LDLM_FL_CBPENDING | LDLM_FL_BL_AST; > unlock_res_and_lock(lock); > @@ -775,14 +770,6 @@ int ldlm_cli_enqueue(struct obd_export *exp, struct > ptlrpc_request **reqp, > body->lock_flags = ldlm_flags_to_wire(*flags); > body->lock_handle[0] = *lockh; > > - /* > - * Liblustre client doesn't get extent locks, except for O_APPEND case > - * where [0, OBD_OBJECT_EOF] lock is taken, or truncate, where > - * [i_size, OBD_OBJECT_EOF] lock is taken. > - */ > - LASSERT(ergo(LIBLUSTRE_CLIENT, einfo->ei_type != LDLM_EXTENT || > - policy->l_extent.end == OBD_OBJECT_EOF)); > - > if (async) { > LASSERT(reqp); > return 0; > -- > 2.7.4 > > ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 13/14] staging/lustre/llite: do not clear uptodate bit in page delete
> From: Jinshan Xiong > > Otherwise, if the race between page fault and truncate occurs, it > will cause the page fault routine to return an EIO error. > > In filemap_fault() { > page_not_uptodate: > ... > ClearPageError(page); > error = mapping->a_ops->readpage(file, page); > if (!error) { > wait_on_page_locked(page); > if (!PageUptodate(page)) > error = -EIO; > } > ... > } > > However, I tend to think this is a defect in kernel implementation, > because it assumes PageUptodate shouldn't be cleared but file read > routine doesn't make the same assumption. Reviewed-by: James Simmons > Signed-off-by: Jinshan Xiong > Reviewed-on: http://review.whamcloud.com/22827 > Intel-bug-id: https://jira.hpdd.intel.com/browse/LU-8633 > Reviewed-by: Li Dongyang > Reviewed-by: Bobi Jam > Signed-off-by: Oleg Drokin > --- > drivers/staging/lustre/lustre/llite/vvp_page.c | 1 - > 1 file changed, 1 deletion(-) > > diff --git a/drivers/staging/lustre/lustre/llite/vvp_page.c > b/drivers/staging/lustre/lustre/llite/vvp_page.c > index 25490a5..23d6630 100644 > --- a/drivers/staging/lustre/lustre/llite/vvp_page.c > +++ b/drivers/staging/lustre/lustre/llite/vvp_page.c > @@ -166,7 +166,6 @@ static void vvp_page_delete(const struct lu_env *env, > refc = atomic_dec_return(&page->cp_ref); > LASSERTF(refc >= 1, "page = %p, refc = %d\n", page, refc); > > - ClearPageUptodate(vmpage); > ClearPagePrivate(vmpage); > vmpage->private = 0; > /* > -- > 2.7.4 > > ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 09/14] staging/lustre/ptlrpc: Suppress error for flock requests
> From: Patrick Farrell > > -EAGAIN is a normal return when requesting POSIX flocks. > We can't recognize exactly that case here, but it's the > only case that should result in -EAGAIN on LDLM_ENQUEUE, so > don't print to console in that case. Reviewed-by: James Simmons > Signed-off-by: Patrick Farrell > Reviewed-on: http://review.whamcloud.com/22856 > Intel-bug-id: https://jira.hpdd.intel.com/browse/LU-8658 > Reviewed-by: Andreas Dilger > Reviewed-by: Bob Glossman > Signed-off-by: Oleg Drokin > --- > drivers/staging/lustre/lustre/ptlrpc/client.c | 4 +++- > 1 file changed, 3 insertions(+), 1 deletion(-) > > diff --git a/drivers/staging/lustre/lustre/ptlrpc/client.c > b/drivers/staging/lustre/lustre/ptlrpc/client.c > index 7cbfb4c..bb7ae4e 100644 > --- a/drivers/staging/lustre/lustre/ptlrpc/client.c > +++ b/drivers/staging/lustre/lustre/ptlrpc/client.c > @@ -1191,7 +1191,9 @@ static int ptlrpc_check_status(struct ptlrpc_request > *req) > lnet_nid_t nid = imp->imp_connection->c_peer.nid; > __u32 opc = lustre_msg_get_opc(req->rq_reqmsg); > > - if (ptlrpc_console_allow(req)) > + /* -EAGAIN is normal when using POSIX flocks */ > + if (ptlrpc_console_allow(req) && > + !(opc == LDLM_ENQUEUE && err == -EAGAIN)) > LCONSOLE_ERROR_MSG(0x011, "%s: operation %s to node %s > failed: rc = %d\n", > imp->imp_obd->obd_name, > ll_opcode2str(opc), > -- > 2.7.4 > > ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 12/14] staging/lustre/llite: update ras window correctly
> From: Bobi Jam > > When stride-RA hit case miss, we only reset normal sequential > read-ahead window, but not reset the stride IO to avoid the overhead > of re-detecting stride IO. While when the normal RA window is set > to not insect with the stride-RA window, when we try to increase > the stride-RA window length later, the presumption does not hold. > > This patch resets the stride IO as well in this case. Reviewed-by: James Simmons > Signed-off-by: Bobi Jam > Reviewed-on: http://review.whamcloud.com/23032 > Intel-bug-id: https://jira.hpdd.intel.com/browse/LU-8683 > Reviewed-by: wangdi > Reviewed-by: Jinshan Xiong > Signed-off-by: Oleg Drokin > --- > drivers/staging/lustre/lustre/llite/rw.c | 15 +++ > 1 file changed, 11 insertions(+), 4 deletions(-) > > diff --git a/drivers/staging/lustre/lustre/llite/rw.c > b/drivers/staging/lustre/lustre/llite/rw.c > index d2515a8..e34017d 100644 > --- a/drivers/staging/lustre/lustre/llite/rw.c > +++ b/drivers/staging/lustre/lustre/llite/rw.c > @@ -809,13 +809,20 @@ static void ras_update(struct ll_sb_info *sbi, struct > inode *inode, > if (ra_miss) { > if (index_in_stride_window(ras, index) && > stride_io_mode(ras)) { > - /*If stride-RA hit cache miss, the stride dector > - *will not be reset to avoid the overhead of > - *redetecting read-ahead mode > - */ > if (index != ras->ras_last_readpage + 1) > ras->ras_consecutive_pages = 0; > ras_reset(inode, ras, index); > + > + /* If stride-RA hit cache miss, the stride > + * detector will not be reset to avoid the > + * overhead of redetecting read-ahead mode, > + * but on the condition that the stride window > + * is still intersect with normal sequential > + * read-ahead window. > + */ > + if (ras->ras_window_start < > + ras->ras_stride_offset) > + ras_stride_reset(ras); > RAS_CDEBUG(ras); > } else { > /* Reset both stride window and normal RA > -- > 2.7.4 > > ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 11/14] staging/lustre/ptlrpc: Correctly calculate hrp->hrp_nthrs
On Wed, 2 Nov 2016, Oleg Drokin wrote: > From: Amir Shehata > > cpu_pattern can specify exactly 1 cpu in a partition: > "0[0]". That means CPT0 will have CPU 0. CPU 0 can have > hyperthreading enabled. This combination would result in > > weight = cfs_cpu_ht_nsiblings(0); > hrp->hrp_nthrs = cfs_cpt_weight(ptlrpc_hr.hr_cpt_table, i); > hrp->hrp_nthrs /= weight; > > evaluating to 0. Where > cfs_cpt_weight(ptlrpc_hr.hr_cpt_table, i) == 1 > weight == 2 > > Therefore, if hrp_nthrs becomes zero, just set it to 1. Reviewed-by: James Simmons > Signed-off-by: Amir Shehata > Reviewed-on: http://review.whamcloud.com/19106 > Intel-bug-id: https://jira.hpdd.intel.com/browse/LU-8492 > Reviewed-by: Liang Zhen > Reviewed-by: Doug Oucharek > Reviewed-by: James Simmons > Signed-off-by: Oleg Drokin > --- > drivers/staging/lustre/lustre/ptlrpc/service.c | 3 ++- > 1 file changed, 2 insertions(+), 1 deletion(-) > > diff --git a/drivers/staging/lustre/lustre/ptlrpc/service.c > b/drivers/staging/lustre/lustre/ptlrpc/service.c > index 72f3930..fc754e7 100644 > --- a/drivers/staging/lustre/lustre/ptlrpc/service.c > +++ b/drivers/staging/lustre/lustre/ptlrpc/service.c > @@ -2541,8 +2541,9 @@ int ptlrpc_hr_init(void) > > hrp->hrp_nthrs = cfs_cpt_weight(ptlrpc_hr.hr_cpt_table, i); > hrp->hrp_nthrs /= weight; > + if (hrp->hrp_nthrs == 0) > + hrp->hrp_nthrs = 1; > > - LASSERT(hrp->hrp_nthrs > 0); > hrp->hrp_thrs = > kzalloc_node(hrp->hrp_nthrs * sizeof(*hrt), GFP_NOFS, >cfs_cpt_spread_node(ptlrpc_hr.hr_cpt_table, > -- > 2.7.4 > > ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH v2] staging: lustre: fixed shadowed variable in socklnd_cb.c
> Removed redundant declaration of variable 'tx' in local scope > Fixed: sparse warning: > socklnd_cb.c:2476:41: warning: symbol 'tx' shadows an earlier one > socklnd_cb.c:2435:25: originally declared here Reviewed-by: James Simmons > Signed-off-by: Andrew Kanner > --- > drivers/staging/lustre/lnet/klnds/socklnd/socklnd_cb.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/drivers/staging/lustre/lnet/klnds/socklnd/socklnd_cb.c > b/drivers/staging/lustre/lnet/klnds/socklnd/socklnd_cb.c > index c1c6f60..f31f4a1 100644 > --- a/drivers/staging/lustre/lnet/klnds/socklnd/socklnd_cb.c > +++ b/drivers/staging/lustre/lnet/klnds/socklnd/socklnd_cb.c > @@ -2473,8 +2473,8 @@ ksocknal_check_peer_timeouts(int idx) >* holding only shared lock >*/ > if (!list_empty(&peer->ksnp_tx_queue)) { > - struct ksock_tx *tx = > list_entry(peer->ksnp_tx_queue.next, > - struct ksock_tx, tx_list); > + tx = list_entry(peer->ksnp_tx_queue.next, > + struct ksock_tx, tx_list); > > if (cfs_time_aftereq(cfs_time_current(), >tx->tx_deadline)) { > -- > 2.1.4 > > ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] staging: lustre: o2iblnd: replace space indentation with tabs
> This patch fixes all CODE_INDENT checkpatch errors in o2iblnd. Reviewed-by: James Simmons > Signed-off-by: Nicholas Hanley > --- > .../staging/lustre/lnet/klnds/o2iblnd/o2iblnd.c| 2 +- > .../staging/lustre/lnet/klnds/o2iblnd/o2iblnd_cb.c | 22 > +++--- > 2 files changed, 12 insertions(+), 12 deletions(-) > > diff --git a/drivers/staging/lustre/lnet/klnds/o2iblnd/o2iblnd.c > b/drivers/staging/lustre/lnet/klnds/o2iblnd/o2iblnd.c > index 9e88021..13235b0 100644 > --- a/drivers/staging/lustre/lnet/klnds/o2iblnd/o2iblnd.c > +++ b/drivers/staging/lustre/lnet/klnds/o2iblnd/o2iblnd.c > @@ -1637,7 +1637,7 @@ int kiblnd_fmr_pool_map(struct kib_fmr_poolset *fps, > struct kib_tx *tx, > { > __u64 *pages = tx->tx_pages; > bool is_rx = (rd != tx->tx_rd); > -bool tx_pages_mapped = 0; > + bool tx_pages_mapped = 0; > struct kib_fmr_pool *fpo; > int npages = 0; > __u64 version; > diff --git a/drivers/staging/lustre/lnet/klnds/o2iblnd/o2iblnd_cb.c > b/drivers/staging/lustre/lnet/klnds/o2iblnd/o2iblnd_cb.c > index b27de88..92692a2 100644 > --- a/drivers/staging/lustre/lnet/klnds/o2iblnd/o2iblnd_cb.c > +++ b/drivers/staging/lustre/lnet/klnds/o2iblnd/o2iblnd_cb.c > @@ -1912,12 +1912,12 @@ kiblnd_close_conn_locked(struct kib_conn *conn, int > error) > libcfs_nid2str(peer->ibp_nid)); > } else { > CNETERR("Closing conn to %s: error %d%s%s%s%s%s\n", > - libcfs_nid2str(peer->ibp_nid), error, > - list_empty(&conn->ibc_tx_queue) ? "" : "(sending)", > - list_empty(&conn->ibc_tx_noops) ? "" : > "(sending_noops)", > - list_empty(&conn->ibc_tx_queue_rsrvd) ? "" : > "(sending_rsrvd)", > - list_empty(&conn->ibc_tx_queue_nocred) ? "" : > "(sending_nocred)", > - list_empty(&conn->ibc_active_txs) ? "" : "(waiting)"); > + libcfs_nid2str(peer->ibp_nid), error, > + list_empty(&conn->ibc_tx_queue) ? "" : "(sending)", > + list_empty(&conn->ibc_tx_noops) ? "" : > "(sending_noops)", > + list_empty(&conn->ibc_tx_queue_rsrvd) ? "" : > "(sending_rsrvd)", > + list_empty(&conn->ibc_tx_queue_nocred) ? "" : > "(sending_nocred)", > + list_empty(&conn->ibc_active_txs) ? "" : "(waiting)"); > } > > dev = ((struct kib_net *)peer->ibp_ni->ni_data)->ibn_dev; > @@ -2643,7 +2643,7 @@ kiblnd_check_reconnect(struct kib_conn *conn, int > version, > if (incarnation) > peer->ibp_incarnation = incarnation; > out: > -write_unlock_irqrestore(glock, flags); > + write_unlock_irqrestore(glock, flags); > > CNETERR("%s: %s (%s), %x, %x, msg_size: %d, queue_depth: %d/%d, > max_frags: %d/%d\n", > libcfs_nid2str(peer->ibp_nid), > @@ -2651,7 +2651,7 @@ kiblnd_check_reconnect(struct kib_conn *conn, int > version, > reason, IBLND_MSG_VERSION, version, msg_size, > conn->ibc_queue_depth, queue_dep, > conn->ibc_max_frags, frag_num); > -/** > + /** >* if conn::ibc_reconnect is TRUE, connd will reconnect to the peer >* while destroying the zombie >*/ > @@ -2976,7 +2976,7 @@ kiblnd_cm_callback(struct rdma_cm_id *cmid, struct > rdma_cm_event *event) > case RDMA_CM_EVENT_ADDR_ERROR: > peer = (struct kib_peer *)cmid->context; > CNETERR("%s: ADDR ERROR %d\n", > - libcfs_nid2str(peer->ibp_nid), event->status); > + libcfs_nid2str(peer->ibp_nid), event->status); > kiblnd_peer_connect_failed(peer, 1, -EHOSTUNREACH); > kiblnd_peer_decref(peer); > return -EHOSTUNREACH; /* rc destroys cmid */ > @@ -3021,7 +3021,7 @@ kiblnd_cm_callback(struct rdma_cm_id *cmid, struct > rdma_cm_event *event) > return kiblnd_active_connect(cmid); > > CNETERR("Can't resolve route for %s: %d\n", > - libcfs_nid2str(peer->ibp_nid), event->status); > + libcfs_nid2str(peer->ibp_nid), event->status); > kiblnd_peer_connect_failed(peer, 1, event->status); > kiblnd_peer_decref(peer); > return event->status; /* rc destroys cmid */ > @@ -3031,7 +3031,7 @@ kiblnd_cm_callback(struct rdma_cm_id *cmid, struct > rdma_cm_event *event) > LASSERT(conn->ibc_state == IBLND_CONN_ACTIVE_CONNECT || > conn->ibc_state == IBLND_CONN_PASSIVE_WAIT); > CNETERR("%s: UNREACHABLE %d\n", > - libcfs_nid2str(conn->ibc_peer->ibp_nid), event->status); > + libcfs_nid2str(conn->ibc_peer->ibp_nid), event->status); > kiblnd_connreq_done(conn, -ENETDOWN); > kiblnd_conn_decref(conn); >
Re: [PATCH] staging: lustre: o2iblnd: replace space indentation with tabs
On Mon, 2016-11-07 at 02:02 +, James Simmons wrote: > > This patch fixes all CODE_INDENT checkpatch errors in o2iblnd. > Reviewed-by: James Simmons [] > > diff --git a/drivers/staging/lustre/lnet/klnds/o2iblnd/o2iblnd.c > > b/drivers/staging/lustre/lnet/klnds/o2iblnd/o2iblnd.c [] > > @@ -1637,7 +1637,7 @@ int kiblnd_fmr_pool_map(struct kib_fmr_poolset *fps, > > struct kib_tx *tx, > > { > > __u64 *pages = tx->tx_pages; > > bool is_rx = (rd != tx->tx_rd); > > -bool tx_pages_mapped = 0; > > + bool tx_pages_mapped = 0; It's generally better to use bool assignments to true/false not 1/0. ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 22/29] staging: lustre: llite: add LL_IOC_FUTIMES_3
> On Thu, Oct 27, 2016 at 06:11:56PM -0400, James Simmons wrote: > > From: John L. Hammond > > > > Add a new regular file ioctl LL_IOC_FUTIMES_3 similar to futimes() but > > which allows setting of all three inode timestamps. Use this ioctl > > during HSM restore to ensure that the volatile file has the same > > timestamps as the file to be restored. Strengthen sanity-hsm test_24a > > to check that archive, release, and restore do not change a file's > > ctime. Add sanity-hsm test_24e to check that tar will succeed when it > > encounters a HSM released file. > > This sounds odd, why is this filesystem the only one that needs a > "special" futimes? Don't make up new syscalls by making an ioctl > please, make a new syscall if that's what you really need! This is a special case dealing with file being retrieve from tape backup. The file being restored ended up with the wrong timestamps. This ioctl was used to retrieve the correct timestamps from our meta data servers. Another difference is that our futimes changes all the timestamps. I see the main issue is wrapping such functionality into ioctls. We have the have developed something, ladvise, along the lines of fadvise. We will have to discuss doing it using syscall instead. ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] staging: lustre: o2iblnd: replace space indentation with tabs
> On Mon, 2016-11-07 at 02:02 +, James Simmons wrote: > > > This patch fixes all CODE_INDENT checkpatch errors in o2iblnd. > > Reviewed-by: James Simmons > [] > > > diff --git a/drivers/staging/lustre/lnet/klnds/o2iblnd/o2iblnd.c > > > b/drivers/staging/lustre/lnet/klnds/o2iblnd/o2iblnd.c > [] > > > @@ -1637,7 +1637,7 @@ int kiblnd_fmr_pool_map(struct kib_fmr_poolset > > > *fps, struct kib_tx *tx, > > > { > > > __u64 *pages = tx->tx_pages; > > > bool is_rx = (rd != tx->tx_rd); > > > -bool tx_pages_mapped = 0; > > > + bool tx_pages_mapped = 0; > > It's generally better to use bool assignments to true/false not 1/0. Nicholas can you resubmit with tx_pages_mapped = false. Looking at the lustre code its the only place like this. Thanks for pointing this out. ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 2/2] staging: lustre: obdclass: Add handling of error returned by lustre_cfg_new
> On Nov 6, 2016, at 12:11 PM, Christophe JAILLET wrote: > > > 'lustre_cfg_new()' can return ERR_PTR(-ENOMEM). > > Handle these errors and propagate the error code to the callers. > > > > Error handling has been rearranged in 'lustre_process_log()' with the > > addition of a label in order to free some resources. > > I wonder if we should just make it return NULL on allocation failure, > and then at least the other error handling that is there (i.e. in your other > patch) > would become correct. > This would make handling in mgc_apply_recover_logs incorrect, but it's already > geared towards this sort of handling anyway, as it discards the passed error > and sets ENOMEM unconditionally (just need to revert 3092c34a in a way). The header lustre_cfg.h is meant to be a UAPI header file. It is used for our userland tools but with the current shape of lustre_cfg.h upstream our tools will not build with it. So having kzalloc and kfree in this header is incorrect. To do this right I need to update our user land tools as well so we should hold off on these patches. ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [lustre-devel] [PATCH 1/2] staging: lustre: replace uses of class_devno_max by MAX_OBD_DEVICES
> On Thu, Nov 3, 2016 at 1:05 AM, Dilger, Andreas > wrote: > On Oct 25, 2016, at 10:47, Aya Mahfouz > wrote: > > > > On Mon, Oct 17, 2016 at 10:38:31PM +, Dilger, Andreas wrote: > >> On Oct 17, 2016, at 15:46, Aya Mahfouz > wrote: > >>> > >>> class_devno_max is an inline function that returns > >>> MAX_OBD_DEVICES. Replace all calls to the function > >>> by MAX_OBD_DEVICES. > >> > >> Thanks for your patch, but unfortunately it can't be accepted. > >> > >> This function was added in preparation of being able to tune the > maximum > >> number of storage devices dynamically, rather than having to hard > code it > >> to the maximum possible number of servers that a client can possibly > >> connect to. > >> > >> While the current maximum of 8192 servers has been enough for current > >> filesystems, I'd rather move in the direction of dynamically > handling this > >> limit rather than re-introducing a hard-coded constant throughout > the code. > >> > > Hello, > > > > I would like to proceed with implementing the function if possible. > > Kindly direct me to some starting pointers. > > Hi Aya, > thanks for offering to look into this. > > There are several ways to approach this problem to make the allocation > of the obd_devs[] array dynamic. In most cases, there isn't any value > to dynamically shrink this array, since the filesystem(s) will typically > be mounted until the node is rebooted, and it is only in the tens of KB > size range, so this will not affect ongoing operations, and that > simplifies > the implementation. > > The easiest way would be to have a dynamically-sized obd_devs[] array > that > is reallocated in class_newdev() in PAGE_SIZE chunks whenever the > current > array has no more free slots and copied to the new array, using > obd_dev_lock > to protect the array while it is being reallocated and copied. In most > cases, this would save memory over the static array (not many > filesystems > have so many servers), but for the few sites that have 1+ servers > they > don't need to change the source to handle this. Using libcfs_kvzalloc() > would avoid issues with allocating large chunks of memory. > > There are a few places where obd_devs[] is accessed outside obd_dev_lock > that would need to be fixed now that this array may be changed at > runtime. > > A second approach that may scale better is to change obd_devs from an > array > to a doubly linked list (using standard list_head helpers). In many > cases > the whole list is seached linearly, and most of the uses of > class_num2obd() > are just used to walk that list in order, which could be replaced with > list_for_each_entry() list traversal. The class_name2dev() function > should > be changed to return the pointer to the obd_device structure, and a new > helper class_dev2num() would just return the obd_minor number from the > obd_device struct for the one use in class_resolve_dev_name(). Using a > linked list has the advantage that there is no need to search for free > slots > in the array, since devices would be removed from the list when it is > freed. > > Cheers, Andreas > > Thanks Andreas! Will start looking into it. Just to let you know I opened a ticket for you. https://jira.hpdd.intel.com/browse/LU-8802 This way wwe can track the progress and have Lustre developers assigened to look at your work. Thanks for stepping forward. > -- > Kind Regards, > Aya Saif El-yazal Mahfouz > > >> One comment inline below, if you still want to submit a patch. > >> > >>> Signed-off-by: Aya Mahfouz > >>> --- > >>> drivers/staging/lustre/lustre/obdclass/class_obd.c | 6 +++--- > >>> drivers/staging/lustre/lustre/obdclass/genops.c | 22 > +++--- > >>> .../lustre/lustre/obdclass/linux/linux-module.c | 6 +++--- > >>> 3 files changed, 17 insertions(+), 17 deletions(-) > >>> > >>> diff --git a/drivers/staging/lustre/lustre/obdclass/class_obd.c > b/drivers/staging/lustre/lustre/obdclass/class_obd.c > >>> index 2b21675..b775c74 100644 > >>> --- a/drivers/staging/lustre/lustre/obdclass/class_obd.c > >>> +++ b/drivers/staging/lustre/lustre/obdclass/class_obd.c > >>> @@ -345,7 +345,7 @@ int class_handle_ioctl(unsigned int cmd, > unsigned long arg) > >>> goto out; > >>> } > >>> obd = class_name2obd(data->ioc_inlbuf4); > >>> - } else if (data->ioc_dev < class_devno_max()) { > >>> + } else if (data->ioc_dev < MAX_OBD_DEVICES) { > >>> obd = class_num2obd(data->ioc_dev); > >>> } else { >
Re: [lustre-devel] [PATCH 1/2] staging: lustre: replace uses of class_devno_max by MAX_OBD_DEVICES
On Nov 4, 2016, at 4:37 AM, Aya Mahfouz wrote: > > On Thu, Nov 3, 2016 at 1:05 AM, Dilger, Andreas > wrote: > On Oct 25, 2016, at 10:47, Aya Mahfouz wrote: > > > > On Mon, Oct 17, 2016 at 10:38:31PM +, Dilger, Andreas wrote: > >> On Oct 17, 2016, at 15:46, Aya Mahfouz > >> wrote: > >>> > >>> class_devno_max is an inline function that returns > >>> MAX_OBD_DEVICES. Replace all calls to the function > >>> by MAX_OBD_DEVICES. > >> > >> Thanks for your patch, but unfortunately it can't be accepted. > >> > >> This function was added in preparation of being able to tune the maximum > >> number of storage devices dynamically, rather than having to hard code it > >> to the maximum possible number of servers that a client can possibly > >> connect to. > >> > >> While the current maximum of 8192 servers has been enough for current > >> filesystems, I'd rather move in the direction of dynamically handling this > >> limit rather than re-introducing a hard-coded constant throughout the code. > >> > > Hello, > > > > I would like to proceed with implementing the function if possible. > > Kindly direct me to some starting pointers. > > Hi Aya, > thanks for offering to look into this. > > There are several ways to approach this problem to make the allocation > of the obd_devs[] array dynamic. In most cases, there isn't any value > to dynamically shrink this array, since the filesystem(s) will typically > be mounted until the node is rebooted, and it is only in the tens of KB > size range, so this will not affect ongoing operations, and that simplifies > the implementation. > > The easiest way would be to have a dynamically-sized obd_devs[] array that > is reallocated in class_newdev() in PAGE_SIZE chunks whenever the current > array has no more free slots and copied to the new array, using obd_dev_lock > to protect the array while it is being reallocated and copied. In most > cases, this would save memory over the static array (not many filesystems > have so many servers), but for the few sites that have 1+ servers they > don't need to change the source to handle this. Using libcfs_kvzalloc() > would avoid issues with allocating large chunks of memory. > > There are a few places where obd_devs[] is accessed outside obd_dev_lock > that would need to be fixed now that this array may be changed at runtime. > > A second approach that may scale better is to change obd_devs from an array > to a doubly linked list (using standard list_head helpers). In many cases > the whole list is seached linearly, and most of the uses of class_num2obd() > are just used to walk that list in order, which could be replaced with > list_for_each_entry() list traversal. The class_name2dev() function should > be changed to return the pointer to the obd_device structure, and a new > helper class_dev2num() would just return the obd_minor number from the > obd_device struct for the one use in class_resolve_dev_name(). Using a > linked list has the advantage that there is no need to search for free slots > in the array, since devices would be removed from the list when it is freed. > > Cheers, Andreas > > Thanks Andreas! Will start looking into it. I also would like to point out that Alexey Lyashkov had an implementation of this in http://review.whamcloud.com/347 but it needed to be reverted as it was way too race-prone in the end. I don't know if Alexey ever improved the patch to actually work (at least there was some talk about it), but even if so, the end result was never contributed back to us. Also please be advised that this is the kind of change that you'll need to have fully functional Lustre setup to verify it works, please let me know if you have any problems setting this up. Thanks! > > -- > Kind Regards, > Aya Saif El-yazal Mahfouz > > >> One comment inline below, if you still want to submit a patch. > >> > >>> Signed-off-by: Aya Mahfouz > >>> --- > >>> drivers/staging/lustre/lustre/obdclass/class_obd.c | 6 +++--- > >>> drivers/staging/lustre/lustre/obdclass/genops.c| 22 > >>> +++--- > >>> .../lustre/lustre/obdclass/linux/linux-module.c| 6 +++--- > >>> 3 files changed, 17 insertions(+), 17 deletions(-) > >>> > >>> diff --git a/drivers/staging/lustre/lustre/obdclass/class_obd.c > >>> b/drivers/staging/lustre/lustre/obdclass/class_obd.c > >>> index 2b21675..b775c74 100644 > >>> --- a/drivers/staging/lustre/lustre/obdclass/class_obd.c > >>> +++ b/drivers/staging/lustre/lustre/obdclass/class_obd.c > >>> @@ -345,7 +345,7 @@ int class_handle_ioctl(unsigned int cmd, unsigned > >>> long arg) > >>> goto out; > >>> } > >>> obd = class_name2obd(data->ioc_inlbuf4); > >>> - } else if (data->ioc_dev < class_devno_max()) { > >>> + } else if (data->ioc_dev < MAX_OBD_DEVICES) { > >>> obd = class_num2obd(data->ioc_dev); > >>> } else { > >>> CERROR("OBD ioctl: No device\n"); > >>> @@ -498,7 +498,7 @@ static int __init ob
Re: [PATCH] staging: lustre: o2iblnd: replace space indentation with tabs
On Mon, Nov 07, 2016 at 03:55:36AM +, James Simmons wrote: > > > On Mon, 2016-11-07 at 02:02 +, James Simmons wrote: > > > > This patch fixes all CODE_INDENT checkpatch errors in o2iblnd. > > > Reviewed-by: James Simmons > > [] > > > > diff --git a/drivers/staging/lustre/lnet/klnds/o2iblnd/o2iblnd.c > > > > b/drivers/staging/lustre/lnet/klnds/o2iblnd/o2iblnd.c > > [] > > > > @@ -1637,7 +1637,7 @@ int kiblnd_fmr_pool_map(struct kib_fmr_poolset > > > > *fps, struct kib_tx *tx, > > > > { > > > > __u64 *pages = tx->tx_pages; > > > > bool is_rx = (rd != tx->tx_rd); > > > > -bool tx_pages_mapped = 0; > > > > + bool tx_pages_mapped = 0; > > > > It's generally better to use bool assignments to true/false not 1/0. > > Nicholas can you resubmit with tx_pages_mapped = false. Looking at the > lustre code its the only place like this. Thanks for pointing this out. No, make it a separate patch, don't change code when you are just changing indentation in the same patch. ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel