devres: provide and use devm_krealloc()
Subsystems (5)
v7 cover devres: provide and use devm_krealloc()
From: Bartosz Golaszewski <[email protected]>
Hi Greg, Andy,
this is another reworked version of devm_krealloc(). This time without the risk of sleeping with spinlock taken but still with enough protection for the devres list. We're using slab's ksize() to determine whether the new size is larger than the current real size of the chunk and then either return the same pointer or manually alloc a new, larger chunk outside of the spinlock.
===
Regular krealloc() obviously can't work with managed memory. This series implements devm_krealloc() and adds two first users with hope that this helper will be adopted by other drivers currently using non-managed krealloc().
v1 -> v2:
- remove leftover call to hwmon_device_unregister() from pmbus_core.c
- add a patch extending devm_kmalloc() to handle zero size case
- use WARN_ON() instead of WARN_ONCE() in devm_krealloc() when passed a pointer to non-managed memory
- correctly handle the case when devm_krealloc() is passed a pointer to memory in .rodata (potentially returned by devm_kstrdup_const())
- correctly handle ZERO_SIZE_PTR passed as the ptr argument in devm_krealloc()
v2 -> v3:
- drop already applied patches
- collect Acks
- add an additional user in iio
v3 -> v4:
- add the kerneldoc for devm_krealloc()
- WARN() outside of spinlock
- rename local variable
v4 -> v5:
- tweak the kerneldoc
v5 -> v6:
- tweak the devres_lock handling in devm_krealloc()
v6 -> v7:
- rework devm_krealloc() to avoid calling krealloc() with spinlock taken
Bartosz Golaszewski (3):
devres: provide devm_krealloc()
hwmon: pmbus: use more devres helpers
iio: adc: xilinx-xadc: use devm_krealloc()
.../driver-api/driver-model/devres.rst | 1 + drivers/base/devres.c | 93 +++++++++++++++++++ drivers/hwmon/pmbus/pmbus_core.c | 28 ++---- drivers/iio/adc/xilinx-xadc-core.c | 16 ++-- include/linux/device.h | 2 + 5 files changed, 113 insertions(+), 27 deletions(-)
Signature · 7 lines
2.26.1 _______________________________________________ linux-arm-kernel mailing list [email protected] http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
PATCH v7 1/3 devres: provide devm_krealloc()
From: Bartosz Golaszewski <[email protected]>
Implement the managed variant of krealloc(). This function works with all memory allocated by devm_kmalloc() (or devres functions using it implicitly like devm_kmemdup(), devm_kstrdup() etc.).
Managed realloc'ed chunks can be manually released with devm_kfree().
--- .../driver-api/driver-model/devres.rst | 1 + drivers/base/devres.c | 93 +++++++++++++++++++ include/linux/device.h | 2 + 3 files changed, 96 insertions(+)
Documentation/driver-api/driver-model/devres.rst +1 -0
@@ -354,6 +354,7 @@ MEM devm_kmalloc() devm_kmalloc_array() devm_kmemdup() + devm_krealloc() devm_kstrdup() devm_kvasprintf() devm_kzalloc()
drivers/base/devres.c +93 -0
111 lines not on this page · raw message
include/linux/device.h +2 -0
@@ -206,6 +206,8 @@ int devres_release_group(struct device *dev, void *id);/* managed devm_k.alloc/kfree for device drivers */ void *devm_kmalloc (struct device *dev,size_t size,gfp_t gfp) __malloc; +void *devm_krealloc (struct device *dev,void *ptr,size_t size, +gfp_t gfp)__must_check ; __printf(3, 0) char *devm_kvasprintf(struct device *dev, gfp_t gfp,const char *fmt,va_list ap) __malloc; __printf(3, 4) char *devm_kasprintf(struct device *dev, gfp_t gfp,
Signature · 7 lines
2.26.1 _______________________________________________ linux-arm-kernel mailing list [email protected] http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
Re: [PATCH v7 1/3] devres: provide devm_krealloc()
Quoted 7 lines · Bartosz Golaszewski
From: Bartosz Golaszewski <[email protected]>
Implement the managed variant of
krealloc(). This function works with all memory allocated bydevm_kmalloc()(or devres functions using it implicitly likedevm_kmemdup(),devm_kstrdup()etc.).Managed realloc'ed chunks can be manually released with
devm_kfree().
Thanks for an update! My comments / questions below.
...
drivers/base/devres.c struct devres { · line 42
Quoted 4 lines
drivers/base/devres.c +4
-0+static struct devres *to_devres (void *data) +{ +return (struct devres *)((u8 *)data -ALIGN (sizeof (struct devres ), +ARCH_KMALLOC_MINALIGN ));
Do you really need both explicit castings?
Quoted 1 line
+}
...
drivers/base/devres.c void *devm_kmalloc(struct device *dev, size_t size, gfp_t gfp) · line 899
Quoted 1 line
drivers/base/devres.c +1
-0+ total_old_size =ksize (to_devres (ptr));
But how you can guarantee this pointer:
- belongs to devres,
- hasn't gone while you run a ksize()?
...
drivers/base/devres.c void *devm_kmalloc(struct device *dev, size_t size, gfp_t gfp) · line 909
Quoted 2 lines
drivers/base/devres.c +2
-0+ new_dr =alloc_dr (devm_kmalloc_release, + total_new_size, gfp,dev_to_node (dev));
Can you move some parameters to the previous line?
Quoted 2 lines
(quoted diff) +2
-0+ if (!new_dr) + return NULL;
...
Quoted 5 lines
drivers/base/devres.c +5
-0+spin_lock_irqsave (&dev->devres_lock, flags); + + old_dr =find_dr (dev, devm_kmalloc_release, devm_kmalloc_match, ptr); +if (!old_dr) { +spin_unlock_irqrestore (&dev->devres_lock, flags);
drivers/base/devres.c void *devm_kmalloc(struct device *dev, size_t size, gfp_t gfp) · line 927
Quoted 10 lines
drivers/base/devres.c +10
-0+devres_free (new_dr); +WARN (1 ,"Memory chunk not managed or managed by a different device." ); +return NULL ; + } + +replace_dr (dev, &old_dr->node, &new_dr->node); + +spin_unlock_irqrestore (&dev->devres_lock, flags); + +memcpy (new_dr->data, old_dr->data,devres_data_size (total_old_size));
But new_dr may concurrently gone at this point, no? It means memcpy() should be under spin lock.
Signature · 2 lines
With Best Regards, Andy Shevchenko
Re: [PATCH v7 1/3] devres: provide devm_krealloc()
Quoted 12 lines · Andy Shevchenko
Quoted 7 lines · Bartosz Golaszewski
From: Bartosz Golaszewski <[email protected]>
Implement the managed variant of
krealloc(). This function works with all memory allocated bydevm_kmalloc()(or devres functions using it implicitly likedevm_kmemdup(),devm_kstrdup()etc.).Managed realloc'ed chunks can be manually released with
devm_kfree().Thanks for an update! My comments / questions below.
Quoted 9 lines
...
drivers/base/devres.c struct devres { · line 42
Quoted 4 lines
drivers/base/devres.c +4
-0+static struct devres *to_devres (void *data) +{ +return (struct devres *)((u8 *)data -ALIGN (sizeof (struct devres ), +ARCH_KMALLOC_MINALIGN ));Do you really need both explicit castings?
Yeah, we can probably drop the (struct devres *) here.
Quoted 8 lines
Quoted 1 line
+}
...
Quoted 1 line
+
total_old_size= ksize(to_devres(ptr));But how you can guarantee this pointer:
- belongs to devres,
We can only check if a chunk is dynamically allocated with ksize() - it will return 0 if it isn't and I'll add a check for that in the next iteration. We check whether it's a managed chunk later after taking the lock.
Quoted 2 lines
- hasn't gone while you run a
ksize()?
At some point you need to draw a line. In the end: how do you guarantee a devres buffer hasn't been freed when you're using it? In my comment to the previous version of this patch I clarified that we need to protect all modifications of the devres linked list - we must not realloc a chunk that contains the links without taking the spinlock but also we must not call alloc() funcs with GFP_KERNEL with spinlock taken. The issue we could run into is: someone modifies the linked list by adding/removing other managed resources, not modifying this one.
The way this function works now guarantees it but other than that:
it's up to the users to not free memory they're actively using.
Quoted 7 lines
...
drivers/base/devres.c void *devm_kmalloc(struct device *dev, size_t size, gfp_t gfp) · line 909
Quoted 2 lines
drivers/base/devres.c +2
-0+ new_dr =alloc_dr (devm_kmalloc_release, + total_new_size, gfp,dev_to_node (dev));Can you move some parameters to the previous line?
Why though? It's fine this way.
Quoted 15 lines
Quoted 2 lines
(quoted diff) +2
-0+ if (!new_dr) + return NULL;...
Quoted 10 lines
drivers/base/devres.c +10
-0+spin_lock_irqsave (&dev->devres_lock, flags); + + old_dr =find_dr (dev, devm_kmalloc_release, devm_kmalloc_match, ptr); +if (!old_dr) { +spin_unlock_irqrestore (&dev->devres_lock, flags); +devres_free (new_dr); +WARN (1 ,"Memory chunk not managed or managed by a different device." ); +return NULL ; + } +
Quoted 9 lines
drivers/base/devres.c void *devm_kmalloc(struct device *dev, size_t size, gfp_t gfp) · line 927
Quoted 5 lines
drivers/base/devres.c +5
-0+replace_dr (dev, &old_dr->node, &new_dr->node); + +spin_unlock_irqrestore (&dev->devres_lock, flags); + +memcpy (new_dr->data, old_dr->data,devres_data_size (total_old_size));But
new_drmay concurrently gone at this point, no? It meansmemcpy()should be under spin lock.
Just as I explained above: we're protecting the linked list, not the resource itself.
Bartosz
Re: [PATCH v7 1/3] devres: provide devm_krealloc()
Quoted 4 lines · Bartosz Golaszewski
Quoted 2 lines · Andy Shevchenko
Quoted 1 line · Bartosz Golaszewski
From: Bartosz Golaszewski <[email protected]>
...
Quoted 9 lines
Quoted 7 lines
drivers/base/devres.c struct devres { · line 42
Quoted 4 lines
drivers/base/devres.c +4
-0+static struct devres *to_devres (void *data) +{ +return (struct devres *)((u8 *)data -ALIGN (sizeof (struct devres ), +ARCH_KMALLOC_MINALIGN ));Do you really need both explicit castings?
Yeah, we can probably drop the (struct devres *) here.
void * -> u8 * here is also not needed, it is considered byte access IIRC.
Quoted 1 line
Quoted 1 line
Quoted 1 line
+}
...
Quoted 12 lines
Quoted 1 line
- hasn't gone while you run a
ksize()?At some point you need to draw a line. In the end: how do you guarantee a devres buffer hasn't been freed when you're using it? In my comment to the previous version of this patch I clarified that we need to protect all modifications of the devres linked list - we must not realloc a chunk that contains the links without taking the spinlock but also we must not call
alloc()funcs withGFP_KERNELwith spinlock taken. The issue we could run into is: someone modifies the linked list by adding/removing other managed resources, not modifying this one.
Quoted 2 lines
The way this function works now guarantees it but other than that:
it's up to the users to not free memory they're actively using.
Thanks for clarification. I agree.
Signature · 2 lines
With Best Regards, Andy Shevchenko
Re: [PATCH v7 1/3] devres: provide devm_krealloc()
Quoted 17 lines · Andy Shevchenko
Quoted 4 lines · Bartosz Golaszewski
Quoted 2 lines · Andy Shevchenko
Quoted 1 line · Bartosz Golaszewski
From: Bartosz Golaszewski <[email protected]>
...
Quoted 8 lines
Quoted 7 lines
drivers/base/devres.c struct devres { · line 42
Quoted 4 lines
drivers/base/devres.c +4
-0+static struct devres *to_devres (void *data) +{ +return (struct devres *)((u8 *)data -ALIGN (sizeof (struct devres ), +ARCH_KMALLOC_MINALIGN ));Do you really need both explicit castings?
Quoted 4 lines
Quoted 1 line
Yeah, we can probably drop the (struct devres *) here.
void * -> u8 * here is also not needed, it is considered byte access IIRC.
Actually it turns out that while we don't need the (void *) -> (u8 *) casting, we must cast to (struct devres *) or the following error is produced:
drivers/base/devres.c: In function ‘to_devres’:drivers/base/devres.c:41:21: error: returning ‘u8 *’ {aka ‘unsigned char *’} from a function with incompatible return type ‘struct devres *’ [-Werror=incompatible-pointer-types]
return ((u8 *)data - ALIGN(sizeof(struct devres),
~~~~~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
ARCH_KMALLOC_MINALIGN));
~~~~~~~~~~~~~~~~~~~~~~~Bart
_______________________________________________
linux-arm-kernel mailing list
[email protected]
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
Re: [PATCH v7 1/3] devres: provide devm_krealloc()
Quoted 28 lines · Bartosz Golaszewski
Quoted 17 lines · Andy Shevchenko
Quoted 4 lines · Bartosz Golaszewski
Quoted 2 lines · Andy Shevchenko
Quoted 1 line · Bartosz Golaszewski
From: Bartosz Golaszewski <[email protected]>
...
Quoted 8 lines
Quoted 7 lines
drivers/base/devres.c struct devres { · line 42
Quoted 4 lines
drivers/base/devres.c +4
-0+static struct devres *to_devres (void *data) +{ +return (struct devres *)((u8 *)data -ALIGN (sizeof (struct devres ), +ARCH_KMALLOC_MINALIGN ));Do you really need both explicit castings?
Quoted 4 lines
Quoted 1 line
Yeah, we can probably drop the (struct devres *) here.
void * -> u8 * here is also not needed, it is considered byte access IIRC.
Actually it turns out that while we don't need the (void *) -> (u8 *) casting, we must cast to (struct devres *) or the following error is produced:
Quoted 8 lines
drivers/base/devres.c: In function ‘to_devres’:drivers/base/devres.c:41:21: error: returning ‘u8 *’ {aka ‘unsigned char *’} from a function with incompatible return type ‘struct devres *’ [-Werror=incompatible-pointer-types]
return ((u8 *)data - ALIGN(sizeof(struct devres),~~~~~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ ARCH_KMALLOC_MINALIGN)); ~~~~~~~~~~~~~~~~~~~~~~~
Of course, you have to drop u8 * casting as well.
Signature · 2 lines
With Best Regards, Andy Shevchenko
Re: [PATCH v7 1/3] devres: provide devm_krealloc()
Quoted 39 lines · Andy Shevchenko
Quoted 28 lines · Bartosz Golaszewski
Quoted 17 lines · Andy Shevchenko
Quoted 4 lines · Bartosz Golaszewski
Quoted 2 lines · Andy Shevchenko
Quoted 1 line · Bartosz Golaszewski
From: Bartosz Golaszewski <[email protected]>
...
Quoted 8 lines
Quoted 7 lines
drivers/base/devres.c struct devres { · line 42
Quoted 4 lines
drivers/base/devres.c +4
-0+static struct devres *to_devres (void *data) +{ +return (struct devres *)((u8 *)data -ALIGN (sizeof (struct devres ), +ARCH_KMALLOC_MINALIGN ));Do you really need both explicit castings?
Quoted 4 lines
Quoted 1 line
Yeah, we can probably drop the (struct devres *) here.
void * -> u8 * here is also not needed, it is considered byte access IIRC.
Actually it turns out that while we don't need the (void *) -> (u8 *) casting, we must cast to (struct devres *) or the following error is produced:
Quoted 8 lines
drivers/base/devres.c: In function ‘to_devres’:drivers/base/devres.c:41:21: error: returning ‘u8 *’ {aka ‘unsigned char *’} from a function with incompatible return type ‘struct devres *’ [-Werror=incompatible-pointer-types]
return ((u8 *)data - ALIGN(sizeof(struct devres),~~~~~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ ARCH_KMALLOC_MINALIGN)); ~~~~~~~~~~~~~~~~~~~~~~~
Quoted 2 lines
Of course, you have to drop u8 * casting as well.
Yes, of course. Duh
Bart
Andy Shevchenko
Reviewed-byRe: [PATCH v7 1/3] devres: provide devm_krealloc()
Quoted 10 lines · Bartosz Golaszewski
Quoted 8 lines · Andy Shevchenko
Quoted 7 lines · Bartosz Golaszewski
Quoted 5 lines · Andy Shevchenko
Quoted 4 lines · Bartosz Golaszewski
Quoted 2 lines · Andy Shevchenko
Quoted 1 line · Bartosz Golaszewski
From: Bartosz Golaszewski <[email protected]>
...
Quoted 26 lines
Quoted 26 lines
Quoted 25 lines
Quoted 8 lines
Quoted 8 lines
Quoted 7 lines
Review
Quoted 4 lines
(quoted diff) +4
-0+static struct devres *to_devres(void *data) +{ + return (struct devres *)((u8 *)data - ALIGN(sizeof(struct devres), + ARCH_KMALLOC_MINALIGN));Do you really need both explicit castings?
Quoted 4 lines
Quoted 1 line
Yeah, we can probably drop the (struct devres *) here.
void * -> u8 * here is also not needed, it is considered byte access IIRC.
Actually it turns out that while we don't need the (void *) -> (u8 *) casting, we must cast to (struct devres *) or the following error is produced:
drivers/base/devres.c: In function ‘to_devres’:drivers/base/devres.c:41:21: error: returning ‘u8 *’ {aka ‘unsigned char *’} from a function with incompatible return type ‘struct devres *’ [-Werror=incompatible-pointer-types]
return ((u8 *)data - ALIGN(sizeof(struct devres),~~~~~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ ARCH_KMALLOC_MINALIGN)); ~~~~~~~~~~~~~~~~~~~~~~~
Quoted 4 lines
Quoted 2 lines
Of course, you have to drop u8 * casting as well.
Yes, of course. Duh
With this addressed (and don't forget to remove also unneeded parentheses),
Thanks!
Signature · 2 lines
With Best Regards, Andy Shevchenko
PATCH v7 2/3 hwmon: pmbus: use more devres helpers
In reply to Bartosz GolaszewskiFrom: Bartosz Golaszewski <[email protected]>
Shrink pmbus code by using devm_hwmon_device_register_with_groups() and devm_krealloc() instead of their non-managed variants.
--- drivers/hwmon/pmbus/pmbus_core.c | 28 +++++++++------------------- 1 file changed, 9 insertions(+), 19 deletions(-)
drivers/hwmon/pmbus/pmbus_core.c +9 -19
@@ -1018,9 +1018,9 @@ static int pmbus_add_attribute(struct pmbus_data *data, struct attribute *attr) {if (data->num_attributes >= data->max_attributes -1 ) {int new_max_attrs = data->max_attributes +PMBUS_ATTR_ALLOC_SIZE ; -void *new_attrs =krealloc (data->group.attrs, - new_max_attrs *sizeof (void *), -GFP_KERNEL ); +void *new_attrs =devm_krealloc (data->dev, data->group.attrs, + new_max_attrs *sizeof (void *), +GFP_KERNEL );if (!new_attrs)return -ENOMEM ; data->group.attrs = new_attrs;
@@ -2534,7 +2534,7 @@ int pmbus_do_probe(struct i2c_client *client, const struct i2c_device_id *id, ret =pmbus_find_attributes (client, data);if (ret) - goto out_kfree; +return ret;/* * If there are no attributes, something is wrong.
@@ -2542,35 +2542,27 @@ int pmbus_do_probe(struct i2c_client *client, const struct i2c_device_id *id,*/ if (!data->num_attributes) {dev_err (dev,"No attributes found\n" ); - ret = -ENODEV ; - goto out_kfree; +return -ENODEV ; } data->groups[0 ] = &data->group;memcpy (data->groups +1 , info->groups,sizeof (void *) * groups_num); - data->hwmon_dev =hwmon_device_register_with_groups (dev, client->name, - data, data->groups); + data->hwmon_dev =devm_hwmon_device_register_with_groups (dev, + client->name, data, data->groups);if (IS_ERR (data->hwmon_dev)) { - ret =PTR_ERR (data->hwmon_dev);dev_err (dev,"Failed to register hwmon device\n" ); - goto out_kfree; +return PTR_ERR (data->hwmon_dev); } ret =pmbus_regulator_register (data);if (ret) - goto out_unregister; +return ret; ret =pmbus_init_debugfs (client, data);if (ret)dev_warn (dev,"Failed to register debugfs\n" );return 0 ; - -out_unregister: -hwmon_device_unregister (data->hwmon_dev); -out_kfree: -kfree (data->group.attrs); -return ret; }EXPORT_SYMBOL_GPL (pmbus_do_probe);
@@ -2580,8 +2572,6 @@ int pmbus_do_remove(struct i2c_client *client)debugfs_remove_recursive (data->debugfs); -hwmon_device_unregister (data->hwmon_dev); -kfree (data->group.attrs);return 0 ; }EXPORT_SYMBOL_GPL (pmbus_do_remove);
Signature · 7 lines
2.26.1 _______________________________________________ linux-arm-kernel mailing list [email protected] http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
PATCH v7 3/3 iio: adc: xilinx-xadc: use devm_krealloc()
In reply to Bartosz GolaszewskiFrom: Bartosz Golaszewski <[email protected]>
Use the managed variant of krealloc() and shrink the code a bit.
--- drivers/iio/adc/xilinx-xadc-core.c | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-)
drivers/iio/adc/xilinx-xadc-core.c +8 -8
@@ -1092,6 +1092,7 @@ MODULE_DEVICE_TABLE(of, xadc_of_match_table);static int xadc_parse_dt (struct iio_dev *indio_dev,struct device_node *np,unsigned int *conf) { +struct device *dev = indio_dev->dev.parent;struct xadc *xadc =iio_priv (indio_dev);struct iio_chan_spec *channels, *chan;struct device_node *chan_node, *child;
@@ -1136,7 +1137,8 @@ static int xadc_parse_dt(struct iio_dev *indio_dev, struct device_node *np, *conf |=XADC_CONF0_MUX |XADC_CONF0_CHAN (ext_mux_chan); } - channels =kmemdup (xadc_channels,sizeof (xadc_channels),GFP_KERNEL ); + channels =devm_kmemdup (dev, xadc_channels, +sizeof (xadc_channels),GFP_KERNEL );if (!channels)return -ENOMEM ;
@@ -1172,8 +1174,9 @@ static int xadc_parse_dt(struct iio_dev *indio_dev, struct device_node *np,of_node_put (chan_node); indio_dev->num_channels = num_channels; - indio_dev->channels =krealloc (channels,sizeof (*channels) * - num_channels,GFP_KERNEL ); + indio_dev->channels =devm_krealloc (dev, channels, +sizeof (*channels) * num_channels, +GFP_KERNEL );/* If we can't resize the channels array, just use the original */ if (!indio_dev->channels) indio_dev->channels = channels;
@@ -1225,14 +1228,14 @@ static int xadc_probe(struct platform_device *pdev) ret =xadc_parse_dt (indio_dev, pdev->dev.of_node, &conf0);if (ret) - goto err_device_free; +return ret;if (xadc->ops->flags &XADC_FLAGS_BUFFERED ) { ret =iio_triggered_buffer_setup (indio_dev, &iio_pollfunc_store_time, &xadc_trigger_handler, &xadc_buffer_ops);if (ret) - goto err_device_free; +return ret; xadc->convst_trigger =xadc_alloc_trigger (indio_dev,"convst" );if (IS_ERR (xadc->convst_trigger)) {
@@ -1350,8 +1353,6 @@ static int xadc_probe(struct platform_device *pdev) err_triggered_buffer_cleanup:if (xadc->ops->flags &XADC_FLAGS_BUFFERED )iio_triggered_buffer_cleanup (indio_dev); -err_device_free: -kfree (indio_dev->channels);return ret; }
@@ -1371,7 +1372,6 @@ static int xadc_remove(struct platform_device *pdev)cancel_delayed_work_sync (&xadc->zynq_unmask_work);clk_disable_unprepare (xadc->clk);kfree (xadc->data); -kfree (indio_dev->channels);return 0 ; }
Signature · 7 lines
2.26.1 _______________________________________________ linux-arm-kernel mailing list [email protected] http://lists.infradead.org/mailman/listinfo/linux-arm-kernel