All of lore.kernel.org
 help / color / mirror / Atom feed
* Re: [Intel-gfx] [igt-dev] [PATCH i-g-t 9/9] i915: Exercise I915_CONTEXT_PARAM_RINGSIZE
From: Chris Wilson @ 2020-02-20 16:00 UTC (permalink / raw)
  To: Janusz Krzysztofik; +Cc: igt-dev, intel-gfx
In-Reply-To: <9103010.GsyPYnKGBp@jkrzyszt-desk.ger.corp.intel.com>

Quoting Janusz Krzysztofik (2020-02-20 15:57:24)
> Hi Chris,
> 
> On Monday, December 2, 2019 3:59:19 PM CET Chris Wilson wrote:
> > Quoting Janusz Krzysztofik (2019-12-02 14:42:58)
> > > Hi Chris,
> > > 
> > > I have a few questions rather than comments.  I hope they are worth spending 
> > > your time.
> > > 
> > > On Wednesday, November 13, 2019 1:52:40 PM CET Chris Wilson wrote:
> > > > I915_CONTEXT_PARAM_RINGSIZE specifies how large to create the command
> > > > ringbuffer for logical ring contects. This directly affects the number
> > > 
> > > s/contects/contexts/
> > > 
> > > > of batches userspace can submit before blocking waiting for space.
> > > > 
> > > > Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> 
> Have you got this patch still queued somewhere?  As UMD has accepted the 
> solution and are ready with changes on their side, I think we need to merge it 
> soon, and the kernel side as well.

Link? That's all I need to merge the kernel...
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply

* Re: [Xen-devel] [PATCH] rwlock: allow recursive read locking when already locked in write mode
From: Jürgen Groß @ 2020-02-20 16:00 UTC (permalink / raw)
  To: Roger Pau Monné, Jan Beulich
  Cc: Stefano Stabellini, Julien Grall, Wei Liu, Konrad Rzeszutek Wilk,
	George Dunlap, Andrew Cooper, Ian Jackson, xen-devel
In-Reply-To: <20200220155729.GP4679@Air-de-Roger>

On 20.02.20 16:57, Roger Pau Monné wrote:
> On Thu, Feb 20, 2020 at 04:50:22PM +0100, Jan Beulich wrote:
>> On 20.02.2020 16:17, Roger Pau Monné wrote:
>>> On Thu, Feb 20, 2020 at 04:02:55PM +0100, Jan Beulich wrote:
>>>> On 20.02.2020 15:11, Roger Pau Monné wrote:
>>>>> On Thu, Feb 20, 2020 at 01:48:54PM +0100, Jan Beulich wrote:
>>>>>> Another option is to use the recurse_cpu field of the
>>>>>> associated spin lock: The field is used for recursive locks
>>>>>> only, and hence the only conflict would be with
>>>>>> _spin_is_locked(), which we don't (and in the future then
>>>>>> also shouldn't) use on this lock.
>>>>>
>>>>> I looked into that also, but things get more complicated AFAICT, as it's
>>>>> not possible to atomically fetch the state of the lock and the owner
>>>>> CPU at the same time. Neither you could set the LOCKED bit and the CPU
>>>>> at the same time.
>>>>
>>>> There's no need to atomically fetch both afaics: The field is
>>>> valid only if the LOCKED bit is set. And when reading the
>>>> field you only care about the value being equal to
>>>> smp_processor_id(), i.e. it is fine to set LOCKED before
>>>> updating the CPU field on lock, and to reset the CPU field to
>>>> SPINLOCK_NO_CPU (or whatever it's called) before clearing
>>>> LOCKED.
>>>
>>> Yes that would require the usage of a sentinel value as 0 would be a
>>> valid processor ID.
>>>
>>> I would try to refrain from (abu)sing internal spinlock fields, as I
>>> think we can solve this just by using the cnts field on the current
>>> rwlock implementation.
>>>
>>> What issue do you have in mind that would prevent storing the CPU
>>> write locked in the cnts field?
>>
>> The reduction of the number of bits used for other purposes.
> 
> I think it should be fine for now, as we would support systems with up
> to 16384 CPU IDs, and 65536 concurrent read lockers, which mean each
> CPU could take the lock up to 4 times recursively. Note that
> supporting 16384 CPUs is still very, very far off the radar.

Current spinlock implementation limits NR_CPUS to 4096.


Juergen

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

^ permalink raw reply

* Re: [PATCH v4 4/6] driver core: Remove driver_deferred_probe_check_state_continue()
From: Bjorn Andersson @ 2020-02-20 15:59 UTC (permalink / raw)
  To: John Stultz
  Cc: lkml, Rob Herring, Rafael J. Wysocki, Kevin Hilman, Ulf Hansson,
	Pavel Machek, Len Brown, Todd Kjos, Liam Girdwood, Mark Brown,
	Thierry Reding, Linus Walleij, Greg Kroah-Hartman, linux-pm
In-Reply-To: <20200220050440.45878-5-john.stultz@linaro.org>

On Wed 19 Feb 21:04 PST 2020, John Stultz wrote:

> Now that driver_deferred_probe_check_state() works better, and
> we've converted the only user of
> driver_deferred_probe_check_state_continue() we can simply
> remove it and simplify some of the logic.
> 
> Cc: Rob Herring <robh@kernel.org>
> Cc: "Rafael J. Wysocki" <rjw@rjwysocki.net>
> Cc: Kevin Hilman <khilman@kernel.org>
> Cc: Ulf Hansson <ulf.hansson@linaro.org>
> Cc: Pavel Machek <pavel@ucw.cz>
> Cc: Len Brown <len.brown@intel.com>
> Cc: Todd Kjos <tkjos@google.com>
> Cc: Bjorn Andersson <bjorn.andersson@linaro.org>
> Cc: Liam Girdwood <lgirdwood@gmail.com>
> Cc: Mark Brown <broonie@kernel.org>
> Cc: Thierry Reding <treding@nvidia.com>
> Cc: Linus Walleij <linus.walleij@linaro.org>
> Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> Cc: linux-pm@vger.kernel.org
> Signed-off-by: John Stultz <john.stultz@linaro.org>
> Change-Id: Id5cd5e9264cfb0fbd70a702715174cc4b10006f4

Change-Id...

Reviewed-by: Bjorn Andersson <bjorn.andersson@linaro.org>

Regards,
Bjorn

> ---
>  drivers/base/dd.c             | 49 +++++------------------------------
>  include/linux/device/driver.h |  1 -
>  2 files changed, 6 insertions(+), 44 deletions(-)
> 
> diff --git a/drivers/base/dd.c b/drivers/base/dd.c
> index fa138f24e2d3..408e4da081da 100644
> --- a/drivers/base/dd.c
> +++ b/drivers/base/dd.c
> @@ -244,19 +244,6 @@ static int __init deferred_probe_timeout_setup(char *str)
>  }
>  __setup("deferred_probe_timeout=", deferred_probe_timeout_setup);
>  
> -static int __driver_deferred_probe_check_state(struct device *dev)
> -{
> -	if (!initcalls_done || deferred_probe_timeout > 0)
> -		return -EPROBE_DEFER;
> -
> -	if (!deferred_probe_timeout) {
> -		dev_WARN(dev, "deferred probe timeout, ignoring dependency");
> -		return -ETIMEDOUT;
> -	}
> -
> -	return 0;
> -}
> -
>  /**
>   * driver_deferred_probe_check_state() - Check deferred probe state
>   * @dev: device to check
> @@ -272,43 +259,19 @@ static int __driver_deferred_probe_check_state(struct device *dev)
>   */
>  int driver_deferred_probe_check_state(struct device *dev)
>  {
> -	int ret;
> +	if (!initcalls_done || deferred_probe_timeout > 0)
> +		return -EPROBE_DEFER;
>  
> -	ret = __driver_deferred_probe_check_state(dev);
> -	if (ret < 0)
> -		return ret;
> +	if (!deferred_probe_timeout) {
> +		dev_WARN(dev, "deferred probe timeout, ignoring dependency");
> +		return -ETIMEDOUT;
> +	}
>  
>  	dev_warn(dev, "ignoring dependency for device, assuming no driver");
>  
>  	return -ENODEV;
>  }
>  
> -/**
> - * driver_deferred_probe_check_state_continue() - check deferred probe state
> - * @dev: device to check
> - *
> - * Returns -ETIMEDOUT if deferred probe debug timeout has expired, or
> - * -EPROBE_DEFER otherwise.
> - *
> - * Drivers or subsystems can opt-in to calling this function instead of
> - * directly returning -EPROBE_DEFER.
> - *
> - * This is similar to driver_deferred_probe_check_state(), but it allows the
> - * subsystem to keep deferring probe after built-in drivers have had a chance
> - * to probe. One scenario where that is useful is if built-in drivers rely on
> - * resources that are provided by modular drivers.
> - */
> -int driver_deferred_probe_check_state_continue(struct device *dev)
> -{
> -	int ret;
> -
> -	ret = __driver_deferred_probe_check_state(dev);
> -	if (ret < 0)
> -		return ret;
> -
> -	return -EPROBE_DEFER;
> -}
> -
>  static void deferred_probe_timeout_work_func(struct work_struct *work)
>  {
>  	struct device_private *private, *p;
> diff --git a/include/linux/device/driver.h b/include/linux/device/driver.h
> index 1188260f9a02..5242afabfaba 100644
> --- a/include/linux/device/driver.h
> +++ b/include/linux/device/driver.h
> @@ -238,7 +238,6 @@ driver_find_device_by_acpi_dev(struct device_driver *drv, const void *adev)
>  
>  void driver_deferred_probe_add(struct device *dev);
>  int driver_deferred_probe_check_state(struct device *dev);
> -int driver_deferred_probe_check_state_continue(struct device *dev);
>  void driver_init(void);
>  
>  /**
> -- 
> 2.17.1
> 

^ permalink raw reply

* Re: [igt-dev] [PATCH i-g-t 9/9] i915: Exercise I915_CONTEXT_PARAM_RINGSIZE
From: Chris Wilson @ 2020-02-20 16:00 UTC (permalink / raw)
  To: Janusz Krzysztofik; +Cc: igt-dev, intel-gfx
In-Reply-To: <9103010.GsyPYnKGBp@jkrzyszt-desk.ger.corp.intel.com>

Quoting Janusz Krzysztofik (2020-02-20 15:57:24)
> Hi Chris,
> 
> On Monday, December 2, 2019 3:59:19 PM CET Chris Wilson wrote:
> > Quoting Janusz Krzysztofik (2019-12-02 14:42:58)
> > > Hi Chris,
> > > 
> > > I have a few questions rather than comments.  I hope they are worth spending 
> > > your time.
> > > 
> > > On Wednesday, November 13, 2019 1:52:40 PM CET Chris Wilson wrote:
> > > > I915_CONTEXT_PARAM_RINGSIZE specifies how large to create the command
> > > > ringbuffer for logical ring contects. This directly affects the number
> > > 
> > > s/contects/contexts/
> > > 
> > > > of batches userspace can submit before blocking waiting for space.
> > > > 
> > > > Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> 
> Have you got this patch still queued somewhere?  As UMD has accepted the 
> solution and are ready with changes on their side, I think we need to merge it 
> soon, and the kernel side as well.

Link? That's all I need to merge the kernel...
-Chris
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

^ permalink raw reply

* [MPTCP] Re: [PATCH mptcp] subflow: place outgoing connections on the join list
From: Paolo Abeni @ 2020-02-20 16:00 UTC (permalink / raw)
  To: mptcp 

[-- Attachment #1: Type: text/plain, Size: 2016 bytes --]

On Tue, 2020-02-04 at 16:35 +0100, Florian Westphal wrote:
> squashto: subflow: place further subflows on new 'join_list'
> 
> in case the subflow can't be established (timeout, reset) the subflow
> socket is leaked.
> 
> Add it to the join list for now.  This isn't nice because the socket
> will be added while its known to not be established, and its not removed
> at any time even if its not useable.
> 
> The list_del_init() call can be removed because the subflow context
> ->node is still unused / not on a list in all cases.
> 
> On a different note:
> 
> I think it makes sense to remove the 'splice join to conn list' approach
> and instead have the worker check the join and conn lists:
> 
> 1. subflows are added from join to conn list once they are established.
> 2. subflows on join list that are still in SYN_SENT are ignored
> 3. subflows on join and/or conn list that are closed already are removed
> and free'd.
> 
> Alternatively, we could probably extend the sk_state_change callback
> to do this and keep checking outside of send/receive code paths.
> 
> Signed-off-by: Florian Westphal <fw(a)strlen.de>
> ---
>  net/mptcp/subflow.c | 5 ++++-
>  1 file changed, 4 insertions(+), 1 deletion(-)
> 
> diff --git a/net/mptcp/subflow.c b/net/mptcp/subflow.c
> index 547d5ffef070..154fb2b42243 100644
> --- a/net/mptcp/subflow.c
> +++ b/net/mptcp/subflow.c
> @@ -821,11 +821,14 @@ int mptcp_subflow_connect(struct sock *sk, struct sockaddr *local,
>  	if (err && err != -EINPROGRESS)
>  		goto failed;
>  
> +	spin_lock_bh(&msk->join_list_lock);
> +	list_add_tail(&subflow->node, &msk->join_list);
> +	spin_unlock_bh(&msk->join_list_lock);
> +
>  	release_sock(sk);
>  	return err;
>  
>  failed:
> -	list_del_init(&subflow->node);
>  	release_sock(sk);
>  	sock_release(sf);
>  	return err;

whooops ... this one fell outside my radar for a long time.

mp_join self-tests (WIP) demonstrated the need for this one!

LGTM, thanks Florian!

/P

^ permalink raw reply

* [PATCH 0/8] Migrate all cros_ec_cmd_xfer() calls to cros_ec_cmd_xfer_status()
From: Enric Balletbo i Serra @ 2020-02-20 15:58 UTC (permalink / raw)
  To: linux-kernel
  Cc: Collabora Kernel ML, groeck, bleung, dtor, gwendal, pmalani,
	Enrico Granata, Andy Shevchenko, Jonathan Cameron, Ting Shen,
	Lee Jones, Neil Armstrong, Dmitry Torokhov, Fei Shao,
	Pi-Hsun Shih, Evan Green, linux-input

Dear all,

The purpose of this series is get rid of the remaining places where the
cros_ec_cmd_xfer() function is used in favour of the
cros_ec_cmd_xfer_status() helper. This allows us to make the
cros_ec_cmd_xfer() function private and only expose to the users a
single way to send commands to the Embedded Controller.

With these changes we also want to help future improvements in the
interface, like the Prashant's series (i.e [1]) to introduce a
cros_ec_cmd() that will allow us to remove more duplicated code in
different places.

Best regards,
 Enric

Note: Prashant, looks like you should fix your sendmail as the patches
      are not threaded.

[1] https://lkml.org/lkml/2020/2/5/614

Enric Balletbo i Serra (8):
  platform/chrome: cros_ec_proto: Report command not supported
  Input: cros_ec_keyb: Use cros_ec_cmd_xfer_status helper
  platform/chrome: cros_ec_vbc: Use cros_ec_cmd_xfer_status helper
  platform/chrome: cros_ec_chardev: Use cros_ec_cmd_xfer_status helper
  platform/chrome: cros_ec_sysfs: Use cros_ec_cmd_xfer_status helper
  platform/chrome: cros_ec_lightbar: Use cros_ec_cmd_xfer_status helper
  platform/chrome: cros_ec: Use cros_ec_cmd_xfer_status helper
  platform/chrome: cros_ec_proto: Do not export cros_ec_cmd_xfer()

 drivers/input/keyboard/cros_ec_keyb.c       | 14 +++---
 drivers/platform/chrome/cros_ec.c           |  2 +-
 drivers/platform/chrome/cros_ec_chardev.c   |  2 +-
 drivers/platform/chrome/cros_ec_lightbar.c  | 50 ++++++---------------
 drivers/platform/chrome/cros_ec_proto.c     | 14 ++++--
 drivers/platform/chrome/cros_ec_sysfs.c     | 36 +++++++--------
 drivers/platform/chrome/cros_ec_vbc.c       |  4 +-
 include/linux/platform_data/cros_ec_proto.h |  3 --
 8 files changed, 50 insertions(+), 75 deletions(-)

-- 
2.25.0


^ permalink raw reply

* [PATCH 2/8] Input: cros_ec_keyb: Use cros_ec_cmd_xfer_status helper
From: Enric Balletbo i Serra @ 2020-02-20 15:58 UTC (permalink / raw)
  To: linux-kernel
  Cc: Collabora Kernel ML, groeck, bleung, dtor, gwendal, pmalani,
	Jonathan Cameron, Ting Shen, Neil Armstrong, Dmitry Torokhov,
	Fei Shao, linux-input
In-Reply-To: <20200220155859.906647-1-enric.balletbo@collabora.com>

This patch makes use of cros_ec_cmd_xfer_status() instead of
cros_ec_cmd_xfer(). In this case there is no advantage of doing this
apart from that we want to make cros_ec_cmd_xfer() a private function
for the EC protocol and let people only use the
cros_ec_cmd_xfer_status() to return Linux standard error codes.

Signed-off-by: Enric Balletbo i Serra <enric.balletbo@collabora.com>
---

 drivers/input/keyboard/cros_ec_keyb.c | 14 +++++---------
 1 file changed, 5 insertions(+), 9 deletions(-)

diff --git a/drivers/input/keyboard/cros_ec_keyb.c b/drivers/input/keyboard/cros_ec_keyb.c
index 2b71c5a51f90..fc1793ca2f17 100644
--- a/drivers/input/keyboard/cros_ec_keyb.c
+++ b/drivers/input/keyboard/cros_ec_keyb.c
@@ -347,18 +347,14 @@ static int cros_ec_keyb_info(struct cros_ec_device *ec_dev,
 	params->info_type = info_type;
 	params->event_type = event_type;
 
-	ret = cros_ec_cmd_xfer(ec_dev, msg);
-	if (ret < 0) {
-		dev_warn(ec_dev->dev, "Transfer error %d/%d: %d\n",
-			 (int)info_type, (int)event_type, ret);
-	} else if (msg->result == EC_RES_INVALID_VERSION) {
+	ret = cros_ec_cmd_xfer_status(ec_dev, msg);
+	if (ret == -ENOTSUPP) {
 		/* With older ECs we just return 0 for everything */
 		memset(result, 0, result_size);
 		ret = 0;
-	} else if (msg->result != EC_RES_SUCCESS) {
-		dev_warn(ec_dev->dev, "Error getting info %d/%d: %d\n",
-			 (int)info_type, (int)event_type, msg->result);
-		ret = -EPROTO;
+	} else if (ret < 0) {
+		dev_warn(ec_dev->dev, "Transfer error %d/%d: %d\n",
+			 (int)info_type, (int)event_type, ret);
 	} else if (ret != result_size) {
 		dev_warn(ec_dev->dev, "Wrong size %d/%d: %d != %zu\n",
 			 (int)info_type, (int)event_type,
-- 
2.25.0


^ permalink raw reply related

* [PATCH 4/8] platform/chrome: cros_ec_chardev: Use cros_ec_cmd_xfer_status helper
From: Enric Balletbo i Serra @ 2020-02-20 15:58 UTC (permalink / raw)
  To: linux-kernel; +Cc: Collabora Kernel ML, groeck, bleung, dtor, gwendal, pmalani
In-Reply-To: <20200220155859.906647-1-enric.balletbo@collabora.com>

This patch makes use of cros_ec_cmd_xfer_status() instead of
cros_ec_cmd_xfer(). In this case the change is trivial and the only
reason to do it is because we want to make cros_ec_cmd_xfer() a private
function for the EC protocol and let people only use the
cros_ec_cmd_xfer_status() to return Linux standard error codes.

Signed-off-by: Enric Balletbo i Serra <enric.balletbo@collabora.com>
---

 drivers/platform/chrome/cros_ec_chardev.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/platform/chrome/cros_ec_chardev.c b/drivers/platform/chrome/cros_ec_chardev.c
index c65e70bc168d..b51ab24055f3 100644
--- a/drivers/platform/chrome/cros_ec_chardev.c
+++ b/drivers/platform/chrome/cros_ec_chardev.c
@@ -301,7 +301,7 @@ static long cros_ec_chardev_ioctl_xcmd(struct cros_ec_dev *ec, void __user *arg)
 	}
 
 	s_cmd->command += ec->cmd_offset;
-	ret = cros_ec_cmd_xfer(ec->ec_dev, s_cmd);
+	ret = cros_ec_cmd_xfer_status(ec->ec_dev, s_cmd);
 	/* Only copy data to userland if data was received. */
 	if (ret < 0)
 		goto exit;
-- 
2.25.0


^ permalink raw reply related

* Re: [PATCH 0/1] selinux: Add xfs quota command types
From: Christoph Hellwig @ 2020-02-20 15:59 UTC (permalink / raw)
  To: Darrick J. Wong; +Cc: Richard Haines, sds, paul, linux-xfs, selinux
In-Reply-To: <20200220155731.GU9506@magnolia>

On Thu, Feb 20, 2020 at 07:57:31AM -0800, Darrick J. Wong wrote:
> On Thu, Feb 20, 2020 at 03:32:33PM +0000, Richard Haines wrote:
> > Added these quota command types for SELinux checks on XFS quotas. I picked
> > those I thought useful. The selinux-testsuite will have tests for these
> > permission checks on XFS.
> > 
> > One point to note: XFS does not call dquot_quota_on() to trigger
> > security_quota_on(), therefore the 'file quotaon' permission is not tested
> > for SELinux
> 
> Is that a feature or a bug? :)
> 
> (It sounds like a bug to me, but let's see if anyone complains...)

The dquot_* routines are not generic quota code, but a specific
implementation used by most non-XFS file systems.  So if there is a bug
it is that the security call is not in the generic dispatch code.

^ permalink raw reply

* [PATCH 6/8] platform/chrome: cros_ec_lightbar: Use cros_ec_cmd_xfer_status helper
From: Enric Balletbo i Serra @ 2020-02-20 15:58 UTC (permalink / raw)
  To: linux-kernel; +Cc: Collabora Kernel ML, groeck, bleung, dtor, gwendal, pmalani
In-Reply-To: <20200220155859.906647-1-enric.balletbo@collabora.com>

This patch makes use of cros_ec_cmd_xfer_status() instead of
cros_ec_cmd_xfer(). It allows us to remove some redundand code. In this
case, though, we are changing a bit the behaviour because of returning
-EINVAL on protocol error we propagate the error return for
cros_ec_cmd_xfer_status() function, but I think it will be fine, even
more clear as we don't mask the Linux error code.

Signed-off-by: Enric Balletbo i Serra <enric.balletbo@collabora.com>
---

 drivers/platform/chrome/cros_ec_lightbar.c | 50 ++++++----------------
 1 file changed, 13 insertions(+), 37 deletions(-)

diff --git a/drivers/platform/chrome/cros_ec_lightbar.c b/drivers/platform/chrome/cros_ec_lightbar.c
index b4c110c5fee0..b59180bff5a3 100644
--- a/drivers/platform/chrome/cros_ec_lightbar.c
+++ b/drivers/platform/chrome/cros_ec_lightbar.c
@@ -116,7 +116,7 @@ static int get_lightbar_version(struct cros_ec_dev *ec,
 
 	param = (struct ec_params_lightbar *)msg->data;
 	param->cmd = LIGHTBAR_CMD_VERSION;
-	ret = cros_ec_cmd_xfer(ec->ec_dev, msg);
+	ret = cros_ec_cmd_xfer_status(ec->ec_dev, msg);
 	if (ret < 0) {
 		ret = 0;
 		goto exit;
@@ -193,15 +193,10 @@ static ssize_t brightness_store(struct device *dev,
 	if (ret)
 		goto exit;
 
-	ret = cros_ec_cmd_xfer(ec->ec_dev, msg);
+	ret = cros_ec_cmd_xfer_status(ec->ec_dev, msg);
 	if (ret < 0)
 		goto exit;
 
-	if (msg->result != EC_RES_SUCCESS) {
-		ret = -EINVAL;
-		goto exit;
-	}
-
 	ret = count;
 exit:
 	kfree(msg);
@@ -258,13 +253,10 @@ static ssize_t led_rgb_store(struct device *dev, struct device_attribute *attr,
 					goto exit;
 			}
 
-			ret = cros_ec_cmd_xfer(ec->ec_dev, msg);
+			ret = cros_ec_cmd_xfer_status(ec->ec_dev, msg);
 			if (ret < 0)
 				goto exit;
 
-			if (msg->result != EC_RES_SUCCESS)
-				goto exit;
-
 			i = 0;
 			ok = 1;
 		}
@@ -305,14 +297,13 @@ static ssize_t sequence_show(struct device *dev,
 	if (ret)
 		goto exit;
 
-	ret = cros_ec_cmd_xfer(ec->ec_dev, msg);
-	if (ret < 0)
-		goto exit;
-
-	if (msg->result != EC_RES_SUCCESS) {
+	ret = cros_ec_cmd_xfer_status(ec->ec_dev, msg);
+	if (ret == -EPROTO) {
 		ret = scnprintf(buf, PAGE_SIZE,
 				"ERROR: EC returned %d\n", msg->result);
 		goto exit;
+	} else if (ret < 0) {
+		goto exit;
 	}
 
 	resp = (struct ec_response_lightbar *)msg->data;
@@ -344,13 +335,10 @@ static int lb_send_empty_cmd(struct cros_ec_dev *ec, uint8_t cmd)
 	if (ret)
 		goto error;
 
-	ret = cros_ec_cmd_xfer(ec->ec_dev, msg);
+	ret = cros_ec_cmd_xfer_status(ec->ec_dev, msg);
 	if (ret < 0)
 		goto error;
-	if (msg->result != EC_RES_SUCCESS) {
-		ret = -EINVAL;
-		goto error;
-	}
+
 	ret = 0;
 error:
 	kfree(msg);
@@ -377,13 +365,10 @@ static int lb_manual_suspend_ctrl(struct cros_ec_dev *ec, uint8_t enable)
 	if (ret)
 		goto error;
 
-	ret = cros_ec_cmd_xfer(ec->ec_dev, msg);
+	ret = cros_ec_cmd_xfer_status(ec->ec_dev, msg);
 	if (ret < 0)
 		goto error;
-	if (msg->result != EC_RES_SUCCESS) {
-		ret = -EINVAL;
-		goto error;
-	}
+
 	ret = 0;
 error:
 	kfree(msg);
@@ -425,15 +410,10 @@ static ssize_t sequence_store(struct device *dev, struct device_attribute *attr,
 	if (ret)
 		goto exit;
 
-	ret = cros_ec_cmd_xfer(ec->ec_dev, msg);
+	ret = cros_ec_cmd_xfer_status(ec->ec_dev, msg);
 	if (ret < 0)
 		goto exit;
 
-	if (msg->result != EC_RES_SUCCESS) {
-		ret = -EINVAL;
-		goto exit;
-	}
-
 	ret = count;
 exit:
 	kfree(msg);
@@ -487,13 +467,9 @@ static ssize_t program_store(struct device *dev, struct device_attribute *attr,
 	 */
 	msg->outsize = count + extra_bytes;
 
-	ret = cros_ec_cmd_xfer(ec->ec_dev, msg);
+	ret = cros_ec_cmd_xfer_status(ec->ec_dev, msg);
 	if (ret < 0)
 		goto exit;
-	if (msg->result != EC_RES_SUCCESS) {
-		ret = -EINVAL;
-		goto exit;
-	}
 
 	ret = count;
 exit:
-- 
2.25.0


^ permalink raw reply related

* [PATCH RESEND v6 06/16] arm64/mm: Use helper fault_signal_pending()
From: Peter Xu @ 2020-02-20 15:59 UTC (permalink / raw)
  To: linux-mm, linux-kernel
  Cc: Peter Xu, Martin Cracauer, Mike Rapoport, Hugh Dickins,
	Jerome Glisse, Kirill A . Shutemov, Matthew Wilcox,
	Pavel Emelyanov, Brian Geffon, Maya Gokhale, Denis Plotnikov,
	Andrea Arcangeli, Johannes Weiner, Dr . David Alan Gilbert,
	Linus Torvalds, Mike Kravetz, Marty McFadden, David Hildenbrand,
	Bobby Powers, Mel Gorman
In-Reply-To: <20200220155353.8676-1-peterx@redhat.com>

Let the arm64 fault handling to use the new fault_signal_pending()
helper, by moving the signal handling out of the retry logic.

Signed-off-by: Peter Xu <peterx@redhat.com>
---
 arch/arm64/mm/fault.c | 19 +++++++------------
 1 file changed, 7 insertions(+), 12 deletions(-)

diff --git a/arch/arm64/mm/fault.c b/arch/arm64/mm/fault.c
index 85566d32958f..6f4b69d712b1 100644
--- a/arch/arm64/mm/fault.c
+++ b/arch/arm64/mm/fault.c
@@ -513,19 +513,14 @@ static int __kprobes do_page_fault(unsigned long addr, unsigned int esr,
 	fault = __do_page_fault(mm, addr, mm_flags, vm_flags);
 	major |= fault & VM_FAULT_MAJOR;
 
-	if (fault & VM_FAULT_RETRY) {
-		/*
-		 * If we need to retry but a fatal signal is pending,
-		 * handle the signal first. We do not need to release
-		 * the mmap_sem because it would already be released
-		 * in __lock_page_or_retry in mm/filemap.c.
-		 */
-		if (fatal_signal_pending(current)) {
-			if (!user_mode(regs))
-				goto no_context;
-			return 0;
-		}
+	/* Quick path to respond to signals */
+	if (fault_signal_pending(fault, regs)) {
+		if (!user_mode(regs))
+			goto no_context;
+		return 0;
+	}
 
+	if (fault & VM_FAULT_RETRY) {
 		/*
 		 * Clear FAULT_FLAG_ALLOW_RETRY to avoid any risk of
 		 * starvation.
-- 
2.24.1



^ permalink raw reply related

* [PATCH 8/8] platform/chrome: cros_ec_proto: Do not export cros_ec_cmd_xfer()
From: Enric Balletbo i Serra @ 2020-02-20 15:58 UTC (permalink / raw)
  To: linux-kernel
  Cc: Collabora Kernel ML, groeck, bleung, dtor, gwendal, pmalani,
	Jonathan Cameron, Enrico Granata, Andy Shevchenko, Lee Jones,
	Pi-Hsun Shih, Evan Green
In-Reply-To: <20200220155859.906647-1-enric.balletbo@collabora.com>

Now that all the remaining users of cros_ec_cmd_xfer() has been removed,
make this function private to the cros_ec_proto module.

Signed-off-by: Enric Balletbo i Serra <enric.balletbo@collabora.com>
---

 drivers/platform/chrome/cros_ec_proto.c     | 5 ++---
 include/linux/platform_data/cros_ec_proto.h | 3 ---
 2 files changed, 2 insertions(+), 6 deletions(-)

diff --git a/drivers/platform/chrome/cros_ec_proto.c b/drivers/platform/chrome/cros_ec_proto.c
index 3e745e0fe092..11a2db7cd0f7 100644
--- a/drivers/platform/chrome/cros_ec_proto.c
+++ b/drivers/platform/chrome/cros_ec_proto.c
@@ -496,8 +496,8 @@ EXPORT_SYMBOL(cros_ec_query_all);
  *
  * Return: 0 on success or negative error code.
  */
-int cros_ec_cmd_xfer(struct cros_ec_device *ec_dev,
-		     struct cros_ec_command *msg)
+static int cros_ec_cmd_xfer(struct cros_ec_device *ec_dev,
+			    struct cros_ec_command *msg)
 {
 	int ret;
 
@@ -541,7 +541,6 @@ int cros_ec_cmd_xfer(struct cros_ec_device *ec_dev,
 
 	return ret;
 }
-EXPORT_SYMBOL(cros_ec_cmd_xfer);
 
 /**
  * cros_ec_cmd_xfer_status() - Send a command to the ChromeOS EC.
diff --git a/include/linux/platform_data/cros_ec_proto.h b/include/linux/platform_data/cros_ec_proto.h
index ba5914770191..1334fedc07cb 100644
--- a/include/linux/platform_data/cros_ec_proto.h
+++ b/include/linux/platform_data/cros_ec_proto.h
@@ -212,9 +212,6 @@ int cros_ec_prepare_tx(struct cros_ec_device *ec_dev,
 int cros_ec_check_result(struct cros_ec_device *ec_dev,
 			 struct cros_ec_command *msg);
 
-int cros_ec_cmd_xfer(struct cros_ec_device *ec_dev,
-		     struct cros_ec_command *msg);
-
 int cros_ec_cmd_xfer_status(struct cros_ec_device *ec_dev,
 			    struct cros_ec_command *msg);
 
-- 
2.25.0


^ permalink raw reply related

* [PATCH] tcg: gdbstub: Fix single-step issue on arm target
From: Changbin Du @ 2020-02-20 15:58 UTC (permalink / raw)
  To: alex.bennee, philmd; +Cc: qemu-devel, Changbin Du

Recently when debugging an arm32 system on qemu, I found sometimes the
single-step command (stepi) is not working. This can be reproduced by
below steps:
 1) start qemu-system-arm -s -S .. and wait for gdb connection.
 2) start gdb and connect to qemu. In my case, gdb gets a wrong value
    (0x60) for PC.
 3) After connected, type 'stepi' and expect it will stop at next ins.

But, it has never stopped. This because:
 1) We doesn't report ‘vContSupported’ feature to gdb explicitly and gdb
    think we do not support it. In this case, gdb use a software breakpoint
    to emulate single-step.
 2) Since gdb gets a wrong initial value of PC, then gdb inserts a
    breakpoint to wrong place (PC+4).

Since we do support ‘vContSupported’ query command, so let's tell gdb that
we support it.

Before this change, gdb send below 'Z0' packet to implement single-step:
gdb_handle_packet: Z0,4,4

After this change, gdb send "vCont;s.." which is expected:
gdb_handle_packet: vCont?
put_packet: vCont;c;C;s;S
gdb_handle_packet: vCont;s:p1.1;c:p1.-1

Signed-off-by: Changbin Du <changbin.du@gmail.com>
---
 gdbstub.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/gdbstub.c b/gdbstub.c
index ce304ff482..adccd938e2 100644
--- a/gdbstub.c
+++ b/gdbstub.c
@@ -2111,7 +2111,7 @@ static void handle_query_supported(GdbCmdContext *gdb_ctx, void *user_ctx)
         gdb_ctx->s->multiprocess = true;
     }
 
-    pstrcat(gdb_ctx->str_buf, sizeof(gdb_ctx->str_buf), ";multiprocess+");
+    pstrcat(gdb_ctx->str_buf, sizeof(gdb_ctx->str_buf), ";vContSupported+;multiprocess+");
     put_packet(gdb_ctx->s, gdb_ctx->str_buf);
 }
 
-- 
2.25.0



^ permalink raw reply related

* [PATCH 5/8] platform/chrome: cros_ec_sysfs: Use cros_ec_cmd_xfer_status helper
From: Enric Balletbo i Serra @ 2020-02-20 15:58 UTC (permalink / raw)
  To: linux-kernel; +Cc: Collabora Kernel ML, groeck, bleung, dtor, gwendal, pmalani
In-Reply-To: <20200220155859.906647-1-enric.balletbo@collabora.com>

This patch makes use of cros_ec_cmd_xfer_status() instead of
cros_ec_cmd_xfer(). In this case the change is trivial and the only
reason to do it is because we want to make cros_ec_cmd_xfer() a private
function for the EC protocol and let people only use the
cros_ec_cmd_xfer_status() to return Linux standard error codes.

Looking at the code I am even unsure that makes sense differentiate
these two errors but let's not change the behaviour for now.

Signed-off-by: Enric Balletbo i Serra <enric.balletbo@collabora.com>
---

 drivers/platform/chrome/cros_ec_sysfs.c | 36 ++++++++++++-------------
 1 file changed, 18 insertions(+), 18 deletions(-)

diff --git a/drivers/platform/chrome/cros_ec_sysfs.c b/drivers/platform/chrome/cros_ec_sysfs.c
index 07dac97ad57c..d45ea5d5bfa4 100644
--- a/drivers/platform/chrome/cros_ec_sysfs.c
+++ b/drivers/platform/chrome/cros_ec_sysfs.c
@@ -149,14 +149,14 @@ static ssize_t version_show(struct device *dev,
 	/* Get build info. */
 	msg->command = EC_CMD_GET_BUILD_INFO + ec->cmd_offset;
 	msg->insize = EC_HOST_PARAM_SIZE;
-	ret = cros_ec_cmd_xfer(ec->ec_dev, msg);
-	if (ret < 0)
-		count += scnprintf(buf + count, PAGE_SIZE - count,
-				   "Build info:    XFER ERROR %d\n", ret);
-	else if (msg->result != EC_RES_SUCCESS)
+	ret = cros_ec_cmd_xfer_status(ec->ec_dev, msg);
+	if (ret == -EPROTO) {
 		count += scnprintf(buf + count, PAGE_SIZE - count,
 				   "Build info:    EC error %d\n", msg->result);
-	else {
+	} else if (ret < 0) {
+		count += scnprintf(buf + count, PAGE_SIZE - count,
+				   "Build info:    XFER ERROR %d\n", ret);
+	} else {
 		msg->data[EC_HOST_PARAM_SIZE - 1] = '\0';
 		count += scnprintf(buf + count, PAGE_SIZE - count,
 				   "Build info:    %s\n", msg->data);
@@ -165,14 +165,14 @@ static ssize_t version_show(struct device *dev,
 	/* Get chip info. */
 	msg->command = EC_CMD_GET_CHIP_INFO + ec->cmd_offset;
 	msg->insize = sizeof(*r_chip);
-	ret = cros_ec_cmd_xfer(ec->ec_dev, msg);
-	if (ret < 0)
-		count += scnprintf(buf + count, PAGE_SIZE - count,
-				   "Chip info:     XFER ERROR %d\n", ret);
-	else if (msg->result != EC_RES_SUCCESS)
+	ret = cros_ec_cmd_xfer_status(ec->ec_dev, msg);
+	if (ret == -EPROTO) {
 		count += scnprintf(buf + count, PAGE_SIZE - count,
 				   "Chip info:     EC error %d\n", msg->result);
-	else {
+	} else if (ret < 0) {
+		count += scnprintf(buf + count, PAGE_SIZE - count,
+				   "Chip info:     XFER ERROR %d\n", ret);
+	} else {
 		r_chip = (struct ec_response_get_chip_info *)msg->data;
 
 		r_chip->vendor[sizeof(r_chip->vendor) - 1] = '\0';
@@ -189,14 +189,14 @@ static ssize_t version_show(struct device *dev,
 	/* Get board version */
 	msg->command = EC_CMD_GET_BOARD_VERSION + ec->cmd_offset;
 	msg->insize = sizeof(*r_board);
-	ret = cros_ec_cmd_xfer(ec->ec_dev, msg);
-	if (ret < 0)
-		count += scnprintf(buf + count, PAGE_SIZE - count,
-				   "Board version: XFER ERROR %d\n", ret);
-	else if (msg->result != EC_RES_SUCCESS)
+	ret = cros_ec_cmd_xfer_status(ec->ec_dev, msg);
+	if (ret == -EPROTO) {
 		count += scnprintf(buf + count, PAGE_SIZE - count,
 				   "Board version: EC error %d\n", msg->result);
-	else {
+	} else if (ret < 0) {
+		count += scnprintf(buf + count, PAGE_SIZE - count,
+				   "Board version: XFER ERROR %d\n", ret);
+	} else {
 		r_board = (struct ec_response_board_version *)msg->data;
 
 		count += scnprintf(buf + count, PAGE_SIZE - count,
-- 
2.25.0


^ permalink raw reply related

* [PATCH 7/8] platform/chrome: cros_ec: Use cros_ec_cmd_xfer_status helper
From: Enric Balletbo i Serra @ 2020-02-20 15:58 UTC (permalink / raw)
  To: linux-kernel; +Cc: Collabora Kernel ML, groeck, bleung, dtor, gwendal, pmalani
In-Reply-To: <20200220155859.906647-1-enric.balletbo@collabora.com>

This patch makes use of cros_ec_cmd_xfer_status() instead of
cros_ec_cmd_xfer(). In this case the change is trivial and the only
reason to do it is because we want to make cros_ec_cmd_xfer() a private
function for the EC protocol and let people only use the
cros_ec_cmd_xfer_status() to return Linux standard error codes.

Signed-off-by: Enric Balletbo i Serra <enric.balletbo@collabora.com>
---

 drivers/platform/chrome/cros_ec.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/platform/chrome/cros_ec.c b/drivers/platform/chrome/cros_ec.c
index 6fc8f2c3ac51..e179411bdfbe 100644
--- a/drivers/platform/chrome/cros_ec.c
+++ b/drivers/platform/chrome/cros_ec.c
@@ -120,7 +120,7 @@ static int cros_ec_sleep_event(struct cros_ec_device *ec_dev, u8 sleep_event)
 
 	buf.msg.command = EC_CMD_HOST_SLEEP_EVENT;
 
-	ret = cros_ec_cmd_xfer(ec_dev, &buf.msg);
+	ret = cros_ec_cmd_xfer_status(ec_dev, &buf.msg);
 
 	/* For now, report failure to transition to S0ix with a warning. */
 	if (ret >= 0 && ec_dev->host_sleep_v1 &&
-- 
2.25.0


^ permalink raw reply related

* [PATCH 3/8] platform/chrome: cros_ec_vbc: Use cros_ec_cmd_xfer_status helper
From: Enric Balletbo i Serra @ 2020-02-20 15:58 UTC (permalink / raw)
  To: linux-kernel; +Cc: Collabora Kernel ML, groeck, bleung, dtor, gwendal, pmalani
In-Reply-To: <20200220155859.906647-1-enric.balletbo@collabora.com>

This patch makes use of cros_ec_cmd_xfer_status() instead of
cros_ec_cmd_xfer(). In this case the change is trivial and the only
reason to do it is because we want to make cros_ec_cmd_xfer() a private
function for the EC protocol and let people only use the
cros_ec_cmd_xfer_status() to return Linux standard error codes.

Signed-off-by: Enric Balletbo i Serra <enric.balletbo@collabora.com>
---

 drivers/platform/chrome/cros_ec_vbc.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/platform/chrome/cros_ec_vbc.c b/drivers/platform/chrome/cros_ec_vbc.c
index 8edae465105c..46482d12cffe 100644
--- a/drivers/platform/chrome/cros_ec_vbc.c
+++ b/drivers/platform/chrome/cros_ec_vbc.c
@@ -40,7 +40,7 @@ static ssize_t vboot_context_read(struct file *filp, struct kobject *kobj,
 	msg->outsize = para_sz;
 	msg->insize = resp_sz;
 
-	err = cros_ec_cmd_xfer(ecdev, msg);
+	err = cros_ec_cmd_xfer_status(ecdev, msg);
 	if (err < 0) {
 		dev_err(dev, "Error sending read request: %d\n", err);
 		kfree(msg);
@@ -83,7 +83,7 @@ static ssize_t vboot_context_write(struct file *filp, struct kobject *kobj,
 	msg->outsize = para_sz;
 	msg->insize = 0;
 
-	err = cros_ec_cmd_xfer(ecdev, msg);
+	err = cros_ec_cmd_xfer_status(ecdev, msg);
 	if (err < 0) {
 		dev_err(dev, "Error sending write request: %d\n", err);
 		kfree(msg);
-- 
2.25.0


^ permalink raw reply related

* [PATCH 1/8] platform/chrome: cros_ec_proto: Report command not supported
From: Enric Balletbo i Serra @ 2020-02-20 15:58 UTC (permalink / raw)
  To: linux-kernel; +Cc: Collabora Kernel ML, groeck, bleung, dtor, gwendal, pmalani
In-Reply-To: <20200220155859.906647-1-enric.balletbo@collabora.com>

In practice most drivers that use the EC protocol what really care is if
the result was successful or not, hence, we introduced a
cros_ec_cmd_xfer_status() function that converts EC errors to standard
Linux error codes. On some few cases, though, we are interested on know
if the command is supported or not, and in such cases, just ignore the
error. To achieve this, return a -ENOTSUPP error when the command is not
supported.

This will allow us to finish the conversion of all users to use the
cros_ec_cmd_xfer_status() function instead of cros_ec_cmd_xfer() and
make the latest private to the protocol driver, so users of the protocol
are not confused in which function they should use.

Signed-off-by: Enric Balletbo i Serra <enric.balletbo@collabora.com>
---

 drivers/platform/chrome/cros_ec_proto.c | 9 ++++++++-
 1 file changed, 8 insertions(+), 1 deletion(-)

diff --git a/drivers/platform/chrome/cros_ec_proto.c b/drivers/platform/chrome/cros_ec_proto.c
index 3cfa643f1d07..3e745e0fe092 100644
--- a/drivers/platform/chrome/cros_ec_proto.c
+++ b/drivers/platform/chrome/cros_ec_proto.c
@@ -553,7 +553,10 @@ EXPORT_SYMBOL(cros_ec_cmd_xfer);
  * replied with success status. It's not necessary to check msg->result when
  * using this function.
  *
- * Return: The number of bytes transferred on success or negative error code.
+ * Return:
+ * >=0 - The number of bytes transferred
+ * -ENOTSUPP - Operation not supported
+ * -EPROTO - Protocol error
  */
 int cros_ec_cmd_xfer_status(struct cros_ec_device *ec_dev,
 			    struct cros_ec_command *msg)
@@ -563,6 +566,10 @@ int cros_ec_cmd_xfer_status(struct cros_ec_device *ec_dev,
 	ret = cros_ec_cmd_xfer(ec_dev, msg);
 	if (ret < 0) {
 		dev_err(ec_dev->dev, "Command xfer error (err:%d)\n", ret);
+	} else if (msg->result == EC_RES_INVALID_VERSION) {
+		dev_dbg(ec_dev->dev, "Command invalid version (err:%d)\n",
+			msg->result);
+		return -ENOTSUPP;
 	} else if (msg->result != EC_RES_SUCCESS) {
 		dev_dbg(ec_dev->dev, "Command result (err: %d)\n", msg->result);
 		return -EPROTO;
-- 
2.25.0


^ permalink raw reply related

* [Xen-devel] [PATCH] MAINTAINERS: make Roger VPCI maintainer
From: Wei Liu @ 2020-02-20 15:58 UTC (permalink / raw)
  To: Xen Development List
  Cc: Stefano Stabellini, Julien Grall, Wei Liu, Konrad Rzeszutek Wilk,
	George Dunlap, Andrew Cooper, Ian Jackson, Jan Beulich,
	Roger Pau Monné

Roger has kindly agreed to take on the burden.

Signed-off-by: Wei Liu <wl@xen.org>
---
 MAINTAINERS | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/MAINTAINERS b/MAINTAINERS
index 46424c1833..e7d1d5a88a 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -500,6 +500,12 @@ F:	xen/include/*/vm_event.h
 F:	xen/include/asm-x86/hvm/monitor.h
 F:	xen/include/asm-x86/hvm/vm_event.h
 
+VPCI
+M:	Roger Pau Monné <roger.pau@citrix.com>
+S:	Supported
+F:	xen/drivers/vpci
+F:	xen/include/xen/vpci.h
+
 VTPM
 M:	Daniel De Graaf <dgdegra@tycho.nsa.gov>
 M:	Quan Xu <quan.xu0@gmail.com>
-- 
2.20.1


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

^ permalink raw reply related

* [PATCH RESEND v6 05/16] arc/mm: Use helper fault_signal_pending()
From: Peter Xu @ 2020-02-20 15:58 UTC (permalink / raw)
  To: linux-mm, linux-kernel
  Cc: Peter Xu, Martin Cracauer, Mike Rapoport, Hugh Dickins,
	Jerome Glisse, Kirill A . Shutemov, Matthew Wilcox,
	Pavel Emelyanov, Brian Geffon, Maya Gokhale, Denis Plotnikov,
	Andrea Arcangeli, Johannes Weiner, Dr . David Alan Gilbert,
	Linus Torvalds, Mike Kravetz, Marty McFadden, David Hildenbrand,
	Bobby Powers, Mel Gorman
In-Reply-To: <20200220155353.8676-1-peterx@redhat.com>

Let ARC to use the new helper fault_signal_pending() by moving the
signal check out of the retry logic as standalone.  This should also
helps to simplify the code a bit.

Signed-off-by: Peter Xu <peterx@redhat.com>
---
 arch/arc/mm/fault.c | 34 +++++++++++++---------------------
 1 file changed, 13 insertions(+), 21 deletions(-)

diff --git a/arch/arc/mm/fault.c b/arch/arc/mm/fault.c
index fb86bc3e9b35..6eb821a59b49 100644
--- a/arch/arc/mm/fault.c
+++ b/arch/arc/mm/fault.c
@@ -133,29 +133,21 @@ void do_page_fault(unsigned long address, struct pt_regs *regs)
 
 	fault = handle_mm_fault(vma, address, flags);
 
+	/* Quick path to respond to signals */
+	if (fault_signal_pending(fault, regs)) {
+		if (!user_mode(regs))
+			goto no_context;
+		return;
+	}
+
 	/*
-	 * Fault retry nuances
+	 * Fault retry nuances, mmap_sem already relinquished by core mm
 	 */
-	if (unlikely(fault & VM_FAULT_RETRY)) {
-
-		/*
-		 * If fault needs to be retried, handle any pending signals
-		 * first (by returning to user mode).
-		 * mmap_sem already relinquished by core mm for RETRY case
-		 */
-		if (fatal_signal_pending(current)) {
-			if (!user_mode(regs))
-				goto no_context;
-			return;
-		}
-		/*
-		 * retry state machine
-		 */
-		if (flags & FAULT_FLAG_ALLOW_RETRY) {
-			flags &= ~FAULT_FLAG_ALLOW_RETRY;
-			flags |= FAULT_FLAG_TRIED;
-			goto retry;
-		}
+	if (unlikely((fault & VM_FAULT_RETRY) &&
+		     (flags & FAULT_FLAG_ALLOW_RETRY))) {
+		flags &= ~FAULT_FLAG_ALLOW_RETRY;
+		flags |= FAULT_FLAG_TRIED;
+		goto retry;
 	}
 
 bad_area:
-- 
2.24.1



^ permalink raw reply related

* Re: [igt-dev] [PATCH i-g-t] intel-ci: add a pre-merge blacklist to reduce the testing queue
From: Chris Wilson @ 2020-02-20 15:58 UTC (permalink / raw)
  To: Martin Peres, igt-dev
In-Reply-To: <20200220153209.210767-1-martin.peres@linux.intel.com>

Quoting Martin Peres (2020-02-20 15:32:09)
> +###############################################################################
> +# These 2 tests are stressing the re-usability of objects. It does not look
> +# like we have had issues with this outside of the gen7 ppgtt issue, which
> +# does not counterbalance its overall execution time.
> +#
> +# - shard-skl: 2% (~5 minutes)
> +# - shard-kbl: 1% (~1.5 minutes)
> +# - shard-apl: 1.7% (~3 minutes)
> +# - shard-glk: 1% (2.5 minutes)
> +# - shard-icl: 0.5% (1 minute)
> +# - shard-tgl: 0.5% (1 minute)
> +#
> +# Data acquired on 2020-02-20 by Martin Peres
> +###############################################################################
> +igt@gem_exec_reuse@(baggage|contexts)

File a bug; there's no reason why this should be greatly exceeding it's
5s runtime. So presumably we waste too much time in the setup/teardown.
-Chris
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

^ permalink raw reply

* Re: [Xen-devel] [PATCH] rwlock: allow recursive read locking when already locked in write mode
From: Roger Pau Monné @ 2020-02-20 15:57 UTC (permalink / raw)
  To: Jan Beulich
  Cc: Jürgen Groß, Stefano Stabellini, Julien Grall, Wei Liu,
	Konrad Rzeszutek Wilk, George Dunlap, Andrew Cooper, Ian Jackson,
	xen-devel
In-Reply-To: <9f82c3cb-862d-87bd-7a01-4812be98eedd@suse.com>

On Thu, Feb 20, 2020 at 04:50:22PM +0100, Jan Beulich wrote:
> On 20.02.2020 16:17, Roger Pau Monné wrote:
> > On Thu, Feb 20, 2020 at 04:02:55PM +0100, Jan Beulich wrote:
> >> On 20.02.2020 15:11, Roger Pau Monné wrote:
> >>> On Thu, Feb 20, 2020 at 01:48:54PM +0100, Jan Beulich wrote:
> >>>> Another option is to use the recurse_cpu field of the
> >>>> associated spin lock: The field is used for recursive locks
> >>>> only, and hence the only conflict would be with
> >>>> _spin_is_locked(), which we don't (and in the future then
> >>>> also shouldn't) use on this lock.
> >>>
> >>> I looked into that also, but things get more complicated AFAICT, as it's
> >>> not possible to atomically fetch the state of the lock and the owner
> >>> CPU at the same time. Neither you could set the LOCKED bit and the CPU
> >>> at the same time.
> >>
> >> There's no need to atomically fetch both afaics: The field is
> >> valid only if the LOCKED bit is set. And when reading the
> >> field you only care about the value being equal to
> >> smp_processor_id(), i.e. it is fine to set LOCKED before
> >> updating the CPU field on lock, and to reset the CPU field to
> >> SPINLOCK_NO_CPU (or whatever it's called) before clearing
> >> LOCKED.
> > 
> > Yes that would require the usage of a sentinel value as 0 would be a
> > valid processor ID.
> > 
> > I would try to refrain from (abu)sing internal spinlock fields, as I
> > think we can solve this just by using the cnts field on the current
> > rwlock implementation.
> > 
> > What issue do you have in mind that would prevent storing the CPU
> > write locked in the cnts field?
> 
> The reduction of the number of bits used for other purposes.

I think it should be fine for now, as we would support systems with up
to 16384 CPU IDs, and 65536 concurrent read lockers, which mean each
CPU could take the lock up to 4 times recursively. Note that
supporting 16384 CPUs is still very, very far off the radar.

Thanks, Roger.

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

^ permalink raw reply

* Re: [PATCH 0/1] selinux: Add xfs quota command types
From: Darrick J. Wong @ 2020-02-20 15:57 UTC (permalink / raw)
  To: Richard Haines; +Cc: sds, paul, linux-xfs, selinux
In-Reply-To: <20200220153234.152426-1-richard_c_haines@btinternet.com>

On Thu, Feb 20, 2020 at 03:32:33PM +0000, Richard Haines wrote:
> Added these quota command types for SELinux checks on XFS quotas. I picked
> those I thought useful. The selinux-testsuite will have tests for these
> permission checks on XFS.
> 
> One point to note: XFS does not call dquot_quota_on() to trigger
> security_quota_on(), therefore the 'file quotaon' permission is not tested
> for SELinux

Is that a feature or a bug? :)

(It sounds like a bug to me, but let's see if anyone complains...)

--D

> Richard Haines (1):
>   selinux: Add xfs quota command types
> 
>  security/selinux/hooks.c | 7 +++++++
>  1 file changed, 7 insertions(+)
> 
> -- 
> 2.24.1
> 

^ permalink raw reply

* Re: [PATCH v7 14/24] btrfs: Convert from readpages to readahead
From: Christoph Hellwig @ 2020-02-20 15:57 UTC (permalink / raw)
  To: Matthew Wilcox
  Cc: cluster-devel@redhat.com, Johannes Thumshirn,
	linux-kernel@vger.kernel.org,
	linux-f2fs-devel@lists.sourceforge.net, Christoph Hellwig,
	linux-mm@kvack.org, ocfs2-devel@oss.oracle.com,
	linux-fsdevel@vger.kernel.org, linux-ext4@vger.kernel.org,
	linux-erofs@lists.ozlabs.org, linux-xfs@vger.kernel.org,
	linux-btrfs@vger.kernel.org
In-Reply-To: <20200220155452.GX24185@bombadil.infradead.org>

On Thu, Feb 20, 2020 at 07:54:52AM -0800, Matthew Wilcox wrote:
> On Thu, Feb 20, 2020 at 07:46:58AM -0800, Christoph Hellwig wrote:
> > On Thu, Feb 20, 2020 at 05:48:49AM -0800, Matthew Wilcox wrote:
> > > btrfs: Convert from readpages to readahead
> > >   
> > > Implement the new readahead method in btrfs.  Add a readahead_page_batch()
> > > to optimise fetching a batch of pages at once.
> > 
> > Shouldn't this readahead_page_batch heper go into a separate patch so
> > that it clearly stands out?
> 
> I'll move it into 'Put readahead pages in cache earlier' for v8 (the
> same patch where we add readahead_page())

One argument for keeping it in a patch of its own is that btrfs appears
to be the only user, and Goldwyn has a WIP conversion of btrfs to iomap,
so it might go away pretty soon and we could just revert the commit.

But this starts to get into really minor details, so I'll shut up now :)

^ permalink raw reply

* Re: [Intel-gfx] [igt-dev] [PATCH i-g-t 9/9] i915: Exercise I915_CONTEXT_PARAM_RINGSIZE
From: Janusz Krzysztofik @ 2020-02-20 15:57 UTC (permalink / raw)
  To: Chris Wilson; +Cc: igt-dev, intel-gfx
In-Reply-To: <157529875955.27263.14886639874940144583@skylake-alporthouse-com>

Hi Chris,

On Monday, December 2, 2019 3:59:19 PM CET Chris Wilson wrote:
> Quoting Janusz Krzysztofik (2019-12-02 14:42:58)
> > Hi Chris,
> > 
> > I have a few questions rather than comments.  I hope they are worth spending 
> > your time.
> > 
> > On Wednesday, November 13, 2019 1:52:40 PM CET Chris Wilson wrote:
> > > I915_CONTEXT_PARAM_RINGSIZE specifies how large to create the command
> > > ringbuffer for logical ring contects. This directly affects the number
> > 
> > s/contects/contexts/
> > 
> > > of batches userspace can submit before blocking waiting for space.
> > > 
> > > Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>

Have you got this patch still queued somewhere?  As UMD has accepted the 
solution and are ready with changes on their side, I think we need to merge it 
soon, and the kernel side as well.

Thanks,
Janusz


> > > ---
> > >  tests/Makefile.sources        |   3 +
> > >  tests/i915/gem_ctx_ringsize.c | 296 ++++++++++++++++++++++++++++++++++
> > >  tests/meson.build             |   1 +
> > >  3 files changed, 300 insertions(+)
> > >  create mode 100644 tests/i915/gem_ctx_ringsize.c
> > > 
> > > diff --git a/tests/Makefile.sources b/tests/Makefile.sources
> > > index e17d43155..801fc52f3 100644
> > > --- a/tests/Makefile.sources
> > > +++ b/tests/Makefile.sources
> > > @@ -163,6 +163,9 @@ gem_ctx_param_SOURCES = i915/gem_ctx_param.c
> > >  TESTS_progs += gem_ctx_persistence
> > >  gem_ctx_persistence_SOURCES = i915/gem_ctx_persistence.c
> > >  
> > > +TESTS_progs += gem_ctx_ringsize
> > > +gem_ctx_ringsize_SOURCES = i915/gem_ctx_ringsize.c
> > > +
> > >  TESTS_progs += gem_ctx_shared
> > >  gem_ctx_shared_SOURCES = i915/gem_ctx_shared.c
> > >  
> > > diff --git a/tests/i915/gem_ctx_ringsize.c b/tests/i915/gem_ctx_ringsize.c
> > > new file mode 100644
> > > index 000000000..1450e8f0d
> > > --- /dev/null
> > > +++ b/tests/i915/gem_ctx_ringsize.c
> > > @@ -0,0 +1,296 @@
> > > +/*
> > > + * Copyright © 2019 Intel Corporation
> > > + *
> > > + * Permission is hereby granted, free of charge, to any person obtaining a
> > > + * copy of this software and associated documentation files (the "Software"),
> > > + * to deal in the Software without restriction, including without limitation
> > > + * the rights to use, copy, modify, merge, publish, distribute, sublicense,
> > > + * and/or sell copies of the Software, and to permit persons to whom the
> > > + * Software is furnished to do so, subject to the following conditions:
> > > + *
> > > + * The above copyright notice and this permission notice (including the next
> > > + * paragraph) shall be included in all copies or substantial portions of the
> > > + * Software.
> > > + *
> > > + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
> > > + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
> > > + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT.  IN NO EVENT SHALL
> > > + * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
> > > + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
> > > + * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS
> > > + * IN THE SOFTWARE.
> > > + */
> > > +
> > > +#include <errno.h>
> > > +#include <fcntl.h>
> > > +#include <inttypes.h>
> > > +#include <sys/ioctl.h>
> > > +#include <sys/types.h>
> > > +#include <unistd.h>
> > > +
> > > +#include "drmtest.h" /* gem_quiescent_gpu()! */
> > > +#include "i915/gem_context.h"
> > > +#include "i915/gem_engine_topology.h"
> > > +#include "ioctl_wrappers.h" /* gem_wait()! */
> > > +#include "sw_sync.h"
> > > +
> > > +#define I915_CONTEXT_PARAM_RINGSIZE 0xc
> > 
> > How are we going to handle symbol redefinition conflict which arises as soon 
> > as this symbol is also included from kernel headers (e.g. via 
> > "i915/gem_engine_topology.h")?
> 
> Final version we copy the headers form the kernel. Conflicts remind us
> when we forget.
> 
> > 
> > > +
> > > +static bool has_ringsize(int i915)
> > > +{
> > > +     struct drm_i915_gem_context_param p = {
> > > +             .param = I915_CONTEXT_PARAM_RINGSIZE,
> > > +     };
> > > +
> > > +     return __gem_context_get_param(i915, &p) == 0;
> > > +}
> > > +
> > > +static void test_idempotent(int i915)
> > > +{
> > > +     struct drm_i915_gem_context_param p = {
> > > +             .param = I915_CONTEXT_PARAM_RINGSIZE,
> > > +     };
> > > +     uint32_t saved;
> > > +
> > > +     /*
> > > +      * Simple test to verify that we are able to read back the same
> > > +      * value as we set.
> > > +      */
> > > +
> > > +     gem_context_get_param(i915, &p);
> > > +     saved = p.value;
> > > +
> > > +     for (uint32_t x = 1 << 12; x <= 128 << 12; x <<= 1) {
> > 
> > I've noticed you are using two different notations for those minimum/maximum 
> > constants.  I think that may be confusing.  How about defining and using 
> > macros?  
> 
> A range in pages...
>  
> > > +             p.value = x;
> > > +             gem_context_set_param(i915, &p);
> > > +             gem_context_get_param(i915, &p);
> > > +             igt_assert_eq_u32(p.value, x);
> > > +     }
> > > +
> > > +     p.value = saved;
> > > +     gem_context_set_param(i915, &p);
> > > +}
> > > +
> > > +static void test_invalid(int i915)
> > > +{
> > > +     struct drm_i915_gem_context_param p = {
> > > +             .param = I915_CONTEXT_PARAM_RINGSIZE,
> > > +     };
> > > +     uint64_t invalid[] = {
> > > +             0, 1, 4095, 4097, 8191, 8193,
> > > +             /* upper limit may be HW dependent, atm it is 512KiB */
> > > +             (512 << 10) - 1, (512 << 10) + 1,
> > 
> > Here is an example of that different notation mentioned above.
> 
> And here written in KiB to match comments.
> 
> > 
> > > +             -1, -1u
> > > +     };
> > > +     uint32_t saved;
> > > +
> > > +     gem_context_get_param(i915, &p);
> > > +     saved = p.value;
> > > +
> > > +     for (int i = 0; i < ARRAY_SIZE(invalid); i++) {
> > > +             p.value = invalid[i];
> > > +             igt_assert_eq(__gem_context_set_param(i915, &p), -EINVAL);
> > > +             gem_context_get_param(i915, &p);
> > > +             igt_assert_eq_u64(p.value, saved);
> > > +     }
> > > +}
> > > +
> > > +static int create_ext_ioctl(int i915,
> > > +                         struct drm_i915_gem_context_create_ext *arg)
> > > +{
> > > +     int err;
> > > +
> > > +     err = 0;
> > > +     if (igt_ioctl(i915, DRM_IOCTL_I915_GEM_CONTEXT_CREATE_EXT, arg)) {
> > > +             err = -errno;
> > > +             igt_assume(err);
> > > +     }
> > > +
> > > +     errno = 0;
> > > +     return err;
> > > +}
> > 
> > This helper looks like pretty standard for me.  Why there are no library 
> > functions for such generic operations?
> 
> Because no one has written that yet.
> 
> > 
> > > +
> > > +static void test_create(int i915)
> > > +{
> > > +     struct drm_i915_gem_context_create_ext_setparam p = {
> > > +             .base = {
> > > +                     .name = I915_CONTEXT_CREATE_EXT_SETPARAM,
> > > +                     .next_extension = 0, /* end of chain */
> > > +             },
> > > +             .param = {
> > > +                     .param = I915_CONTEXT_PARAM_RINGSIZE,
> > > +                     .value = 512 << 10,
> > > +             }
> > > +     };
> > > +     struct drm_i915_gem_context_create_ext create = {
> > > +             .flags = I915_CONTEXT_CREATE_FLAGS_USE_EXTENSIONS,
> > > +             .extensions = to_user_pointer(&p),
> > > +     };
> > > +
> > > +     igt_assert_eq(create_ext_ioctl(i915, &create),  0);
> > > +
> > > +     p.param.ctx_id = create.ctx_id;
> > > +     p.param.value = 0;
> > > +     gem_context_get_param(i915, &p.param);
> > > +     igt_assert_eq(p.param.value, 512 << 10);
> > > +
> > > +     gem_context_destroy(i915, create.ctx_id);
> > > +}
> > > +
> > > +static void test_clone(int i915)
> > > +{
> > > +     struct drm_i915_gem_context_create_ext_setparam p = {
> > > +             .base = {
> > > +                     .name = I915_CONTEXT_CREATE_EXT_SETPARAM,
> > > +                     .next_extension = 0, /* end of chain */
> > > +             },
> > > +             .param = {
> > > +                     .param = I915_CONTEXT_PARAM_RINGSIZE,
> > > +                     .value = 512 << 10,
> > > +             }
> > > +     };
> > > +     struct drm_i915_gem_context_create_ext create = {
> > > +             .flags = I915_CONTEXT_CREATE_FLAGS_USE_EXTENSIONS,
> > > +             .extensions = to_user_pointer(&p),
> > > +     };
> > > +
> > > +     igt_assert_eq(create_ext_ioctl(i915, &create),  0);
> > > +
> > > +     p.param.ctx_id = gem_context_clone(i915, create.ctx_id,
> > > +                                        I915_CONTEXT_CLONE_ENGINES, 0);
> > > +     igt_assert_neq(p.param.ctx_id, create.ctx_id);
> > > +     gem_context_destroy(i915, create.ctx_id);
> > > +
> > > +     p.param.value = 0;
> > > +     gem_context_get_param(i915, &p.param);
> > > +     igt_assert_eq(p.param.value, 512 << 10);
> > > +
> > > +     gem_context_destroy(i915, p.param.ctx_id);
> > > +}
> > > +
> > > +static int __execbuf(int i915, struct drm_i915_gem_execbuffer2 *execbuf)
> > > +{
> > > +     int err;
> > > +
> > > +     err = 0;
> > > +     if (ioctl(i915, DRM_IOCTL_I915_GEM_EXECBUFFER2, execbuf))
> > > +             err = -errno;
> > > +
> > > +     errno = 0;
> > > +     return err;
> > > +}
> > 
> > The above helper looks pretty the same as lib/ioctlwrappers.c:__gem_execbuf().  
> > Does igt_assume(err) found in the latter matter so much that you use your own 
> > version?
> 
> It's very, very different from that one.
> 
> > > +
> > > +static uint32_t __batch_create(int i915, uint32_t offset)
> > 
> > This is always called with offset = 0, do we expect other values to be used 
> > later?
> 
> Why not.
>  
> > > +{
> > > +     const uint32_t bbe = 0xa << 23;
> > > +     uint32_t handle;
> > > +
> > > +     handle = gem_create(i915, ALIGN(offset + sizeof(bbe), 4096));
> > 
> > Why don't we rely on the driver making the alignment for us?
> 
> I'm used to being inside the kernel where it's expected to be correct.
> 
> > > +     gem_write(i915, handle, offset, &bbe, sizeof(bbe));
> > > +
> > > +     return handle;
> > > +}
> > > +
> > > +static uint32_t batch_create(int i915)
> > > +{
> > > +     return __batch_create(i915, 0);
> > > +}
> > > +
> > > +static unsigned int measure_inflight(int i915, unsigned int engine)
> > > +{
> > > +     IGT_CORK_FENCE(cork);
> > > +     struct drm_i915_gem_exec_object2 obj = {
> > > +             .handle = batch_create(i915)
> > > +     };
> > > +     struct drm_i915_gem_execbuffer2 execbuf = {
> > > +             .buffers_ptr = to_user_pointer(&obj),
> > > +             .buffer_count = 1,
> > > +             .flags = engine | I915_EXEC_FENCE_IN,
> > > +             .rsvd2 = igt_cork_plug(&cork, i915),
> > > +     };
> > > +     unsigned int count;
> > > +
> > > +     fcntl(i915, F_SETFL, fcntl(i915, F_GETFL) | O_NONBLOCK);
> > > +
> > > +     gem_execbuf(i915, &execbuf);
> > > +     for (count = 1; __execbuf(i915, &execbuf) == 0; count++)
> > > +             ;
> > 
> > Shouldn't we check if the reason for the failure is what we expect, i.e., 
> > -EWOULDBLOCK (or -EINTR)?  And why don't we put a time constraint on that loop 
> > in case O_NONBLOCK handling is not supported (yet)?
> 
> Sure. The idea is that O_NONBLOCK is supported, otherwise we don't
> have fast and precise feedback.
> 
> > > +static void test_resize(int i915,
> > > +                     const struct intel_execution_engine2 *e,
> > > +                     unsigned int flags)
> > > +#define IDLE (1 << 0)
> > > +{
> > > +     struct drm_i915_gem_context_param p = {
> > > +             .param = I915_CONTEXT_PARAM_RINGSIZE,
> > > +     };
> > > +     unsigned int prev[2] = {};
> > > +     uint32_t saved;
> > > +
> > > +     gem_context_get_param(i915, &p);
> > > +     saved = p.value;
> > > +
> > > +     gem_quiescent_gpu(i915);
> > > +     for (p.value = 1 << 12; p.value <= 128 << 12; p.value <<= 1) {
> > > +             unsigned int count;
> > > +
> > > +             gem_context_set_param(i915, &p);
> > > +
> > > +             count = measure_inflight(i915, e->flags);
> > > +             igt_info("%s: %llx -> %d\n", e->name, p.value, count);
> > > +             igt_assert(count > 3 * (prev[1] - prev[0]) / 4 + prev[1]);
> > 
> > Where does this formula come from?  Why not just count == 2 * prev[1] ?
> > What results should we expect in "active" vs. "idle" mode?
> 
> I've explained somewhere why it is not 2*prev... And there's a small
> amount of imprecision (+-1 request). In test_resize is the comment:
> 
>         /*
>          * The ringsize directly affects the number of batches we can have
>          * inflight -- when we run out of room in the ring, the client is
>          * blocked (or if O_NONBLOCK is specified, -EWOULDBLOCK is reported).
>          * The kernel throttles the client when they enter the last 4KiB page,
>          * so as we double the size of the ring, we nearly double the number
>          * of requests we can fit as 2^n-1: i.e 0, 1, 3, 7, 15, 31 pages.
>          */
> 
> -Chris
> 




_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply

* Re: [PATCH v7 14/24] btrfs: Convert from readpages to readahead
From: Christoph Hellwig @ 2020-02-20 15:57 UTC (permalink / raw)
  To: Matthew Wilcox
  Cc: Christoph Hellwig, Johannes Thumshirn,
	linux-fsdevel@vger.kernel.org, linux-mm@kvack.org,
	linux-kernel@vger.kernel.org, linux-btrfs@vger.kernel.org,
	linux-erofs@lists.ozlabs.org, linux-ext4@vger.kernel.org,
	linux-f2fs-devel@lists.sourceforge.net, cluster-devel@redhat.com,
	ocfs2-devel@oss.oracle.com, linux-xfs@vger.kernel.org
In-Reply-To: <20200220155452.GX24185@bombadil.infradead.org>

On Thu, Feb 20, 2020 at 07:54:52AM -0800, Matthew Wilcox wrote:
> On Thu, Feb 20, 2020 at 07:46:58AM -0800, Christoph Hellwig wrote:
> > On Thu, Feb 20, 2020 at 05:48:49AM -0800, Matthew Wilcox wrote:
> > > btrfs: Convert from readpages to readahead
> > >   
> > > Implement the new readahead method in btrfs.  Add a readahead_page_batch()
> > > to optimise fetching a batch of pages at once.
> > 
> > Shouldn't this readahead_page_batch heper go into a separate patch so
> > that it clearly stands out?
> 
> I'll move it into 'Put readahead pages in cache earlier' for v8 (the
> same patch where we add readahead_page())

One argument for keeping it in a patch of its own is that btrfs appears
to be the only user, and Goldwyn has a WIP conversion of btrfs to iomap,
so it might go away pretty soon and we could just revert the commit.

But this starts to get into really minor details, so I'll shut up now :)

^ permalink raw reply


This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.