Introduce devm_add() to factorize devres_alloc/devres_add calls.
Using that helper produces simpler code and smaller object size:
1 file changed, 27 insertions(+), 66 deletions(-)
text data bss dec hex filename
- 1708 80 0 1788 6fc drivers/clk/clk-devres.o
+ 1508 80 0 1588 634 drivers/clk/clk-devres.o
Signed-off-by: Marc Gonzalez <[email protected]>
---
drivers/base/devres.c | 14 ++++++
drivers/clk/clk-devres.c | 93 ++++++++++++----------------------------
include/linux/device.h | 1 +
3 files changed, 42 insertions(+), 66 deletions(-)
diff --git a/drivers/base/devres.c b/drivers/base/devres.c
index 0bbb328bd17f..76392dd6273b 100644
--- a/drivers/base/devres.c
+++ b/drivers/base/devres.c
@@ -685,6 +685,20 @@ int devres_release_group(struct device *dev, void *id)
}
EXPORT_SYMBOL_GPL(devres_release_group);
+void *devm_add(struct device *dev, dr_release_t func, void *arg, size_t size)
+{
+ void *data = devres_alloc(func, size, GFP_KERNEL);
+
+ if (data) {
+ memcpy(data, arg, size);
+ devres_add(dev, data);
+ } else
+ func(dev, arg);
+
+ return data;
+}
+EXPORT_SYMBOL_GPL(devm_add);
+
/*
* Custom devres actions allow inserting a simple function call
* into the teadown sequence.
diff --git a/drivers/clk/clk-devres.c b/drivers/clk/clk-devres.c
index be160764911b..582fda9ad6a6 100644
--- a/drivers/clk/clk-devres.c
+++ b/drivers/clk/clk-devres.c
@@ -4,26 +4,18 @@
#include <linux/export.h>
#include <linux/gfp.h>
-static void devm_clk_release(struct device *dev, void *res)
+static void wrap_clk_put(struct device *dev, void *res)
{
clk_put(*(struct clk **)res);
}
struct clk *devm_clk_get(struct device *dev, const char *id)
{
- struct clk **ptr, *clk;
-
- ptr = devres_alloc(devm_clk_release, sizeof(*ptr), GFP_KERNEL);
- if (!ptr)
- return ERR_PTR(-ENOMEM);
-
- clk = clk_get(dev, id);
- if (!IS_ERR(clk)) {
- *ptr = clk;
- devres_add(dev, ptr);
- } else {
- devres_free(ptr);
- }
+ struct clk *clk = clk_get(dev, id);
+
+ if (!IS_ERR(clk))
+ if (!devm_add(dev, wrap_clk_put, &clk, sizeof(clk)))
+ clk = ERR_PTR(-ENOMEM);
return clk;
}
@@ -33,10 +25,7 @@ struct clk *devm_clk_get_optional(struct device *dev, const char *id)
{
struct clk *clk = devm_clk_get(dev, id);
- if (clk == ERR_PTR(-ENOENT))
- return NULL;
-
- return clk;
+ return clk == ERR_PTR(-ENOENT) ? NULL : clk;
}
EXPORT_SYMBOL(devm_clk_get_optional);
@@ -45,7 +34,7 @@ struct clk_bulk_devres {
int num_clks;
};
-static void devm_clk_bulk_release(struct device *dev, void *res)
+static void wrap_clk_bulk_put(struct device *dev, void *res)
{
struct clk_bulk_devres *devres = res;
@@ -55,25 +44,17 @@ static void devm_clk_bulk_release(struct device *dev, void *res)
static int __devm_clk_bulk_get(struct device *dev, int num_clks,
struct clk_bulk_data *clks, bool optional)
{
- struct clk_bulk_devres *devres;
int ret;
-
- devres = devres_alloc(devm_clk_bulk_release,
- sizeof(*devres), GFP_KERNEL);
- if (!devres)
- return -ENOMEM;
+ struct clk_bulk_devres arg = { clks, num_clks };
if (optional)
ret = clk_bulk_get_optional(dev, num_clks, clks);
else
ret = clk_bulk_get(dev, num_clks, clks);
- if (!ret) {
- devres->clks = clks;
- devres->num_clks = num_clks;
- devres_add(dev, devres);
- } else {
- devres_free(devres);
- }
+
+ if (!ret)
+ if (!devm_add(dev, wrap_clk_bulk_put, &arg, sizeof(arg)))
+ ret = -ENOMEM;
return ret;
}
@@ -95,24 +76,16 @@ EXPORT_SYMBOL_GPL(devm_clk_bulk_get_optional);
int __must_check devm_clk_bulk_get_all(struct device *dev,
struct clk_bulk_data **clks)
{
- struct clk_bulk_devres *devres;
- int ret;
+ struct clk_bulk_devres arg;
- devres = devres_alloc(devm_clk_bulk_release,
- sizeof(*devres), GFP_KERNEL);
- if (!devres)
- return -ENOMEM;
-
- ret = clk_bulk_get_all(dev, &devres->clks);
- if (ret > 0) {
- *clks = devres->clks;
- devres->num_clks = ret;
- devres_add(dev, devres);
- } else {
- devres_free(devres);
- }
+ arg.num_clks = clk_bulk_get_all(dev, clks);
+ arg.clks = *clks;
- return ret;
+ if (arg.num_clks > 0)
+ if (!devm_add(dev, wrap_clk_bulk_put, &arg, sizeof(arg)))
+ arg.num_clks = -ENOMEM;
+
+ return arg.num_clks;
}
EXPORT_SYMBOL_GPL(devm_clk_bulk_get_all);
@@ -128,30 +101,18 @@ static int devm_clk_match(struct device *dev, void *res, void *data)
void devm_clk_put(struct device *dev, struct clk *clk)
{
- int ret;
-
- ret = devres_release(dev, devm_clk_release, devm_clk_match, clk);
-
- WARN_ON(ret);
+ WARN_ON(devres_release(dev, wrap_clk_put, devm_clk_match, clk));
}
EXPORT_SYMBOL(devm_clk_put);
struct clk *devm_get_clk_from_child(struct device *dev,
struct device_node *np, const char *con_id)
{
- struct clk **ptr, *clk;
-
- ptr = devres_alloc(devm_clk_release, sizeof(*ptr), GFP_KERNEL);
- if (!ptr)
- return ERR_PTR(-ENOMEM);
-
- clk = of_clk_get_by_name(np, con_id);
- if (!IS_ERR(clk)) {
- *ptr = clk;
- devres_add(dev, ptr);
- } else {
- devres_free(ptr);
- }
+ struct clk *clk = of_clk_get_by_name(np, con_id);
+
+ if (!IS_ERR(clk))
+ if (!devm_add(dev, wrap_clk_put, &clk, sizeof(clk)))
+ clk = ERR_PTR(-ENOMEM);
return clk;
}
diff --git a/include/linux/device.h b/include/linux/device.h
index e226030c1df3..d47872c5ab72 100644
--- a/include/linux/device.h
+++ b/include/linux/device.h
@@ -969,6 +969,7 @@ void __iomem *devm_of_iomap(struct device *dev,
struct device_node *node, int index,
resource_size_t *size);
+void *devm_add(struct device *dev, dr_release_t func, void *arg, size_t size);
/* allows to add/remove a custom action to devres stack */
int devm_add_action(struct device *dev, void (*action)(void *), void *data);
void devm_remove_action(struct device *dev, void (*action)(void *), void *data);
--
2.17.1
Hi Marc,
On Wed, Jan 22, 2020 at 2:02 PM Marc Gonzalez <[email protected]> wrote:
> Introduce devm_add() to factorize devres_alloc/devres_add calls.
>
> Using that helper produces simpler code and smaller object size:
>
> 1 file changed, 27 insertions(+), 66 deletions(-)
>
> text data bss dec hex filename
> - 1708 80 0 1788 6fc drivers/clk/clk-devres.o
> + 1508 80 0 1588 634 drivers/clk/clk-devres.o
>
> Signed-off-by: Marc Gonzalez <[email protected]>
Thanks for your patch!
> --- a/drivers/base/devres.c
> +++ b/drivers/base/devres.c
> @@ -685,6 +685,20 @@ int devres_release_group(struct device *dev, void *id)
> }
> EXPORT_SYMBOL_GPL(devres_release_group);
>
> +void *devm_add(struct device *dev, dr_release_t func, void *arg, size_t size)
I there any advantage of using dr_release_t over "void (*action)(void *)",
like devm_add_action() does? The latter lacks the "device *" parameter.
> +{
> + void *data = devres_alloc(func, size, GFP_KERNEL);
> +
> + if (data) {
> + memcpy(data, arg, size);
> + devres_add(dev, data);
> + } else
> + func(dev, arg);
Both branchs should use { ...}
> +
> + return data;
Why return data or NULL, instead of 0 or -Efoo, like devm_add_action()?
> +}
> +EXPORT_SYMBOL_GPL(devm_add);
> +
> /*
> * Custom devres actions allow inserting a simple function call
> * into the teadown sequence.
> diff --git a/drivers/clk/clk-devres.c b/drivers/clk/clk-devres.c
> index be160764911b..582fda9ad6a6 100644
> --- a/drivers/clk/clk-devres.c
> +++ b/drivers/clk/clk-devres.c
> @@ -33,10 +25,7 @@ struct clk *devm_clk_get_optional(struct device *dev, const char *id)
> {
> struct clk *clk = devm_clk_get(dev, id);
>
> - if (clk == ERR_PTR(-ENOENT))
> - return NULL;
> -
> - return clk;
> + return clk == ERR_PTR(-ENOENT) ? NULL : clk;
Unrelated change (which is less readable than the original, IMHO).
> }
> EXPORT_SYMBOL(devm_clk_get_optional);
>
> @@ -45,7 +34,7 @@ struct clk_bulk_devres {
> int num_clks;
> };
>
> -static void devm_clk_bulk_release(struct device *dev, void *res)
> +static void wrap_clk_bulk_put(struct device *dev, void *res)
> {
> struct clk_bulk_devres *devres = res;
>
> @@ -55,25 +44,17 @@ static void devm_clk_bulk_release(struct device *dev, void *res)
> static int __devm_clk_bulk_get(struct device *dev, int num_clks,
> struct clk_bulk_data *clks, bool optional)
> {
> - struct clk_bulk_devres *devres;
> int ret;
> -
> - devres = devres_alloc(devm_clk_bulk_release,
> - sizeof(*devres), GFP_KERNEL);
> - if (!devres)
> - return -ENOMEM;
> + struct clk_bulk_devres arg = { clks, num_clks };
>
> if (optional)
> ret = clk_bulk_get_optional(dev, num_clks, clks);
> else
> ret = clk_bulk_get(dev, num_clks, clks);
> - if (!ret) {
> - devres->clks = clks;
> - devres->num_clks = num_clks;
> - devres_add(dev, devres);
> - } else {
> - devres_free(devres);
> - }
> +
> + if (!ret)
> + if (!devm_add(dev, wrap_clk_bulk_put, &arg, sizeof(arg)))
> + ret = -ENOMEM;
Nested ifs are easier to read when the outer one uses curly braces:
if (!ret) {
if (!devm_add(dev, wrap_clk_bulk_put, &arg, sizeof(arg)))
ret = -ENOMEM;
}
Or merge the condition with &&.
>
> return ret;
But in this case, I would write it as:
if (ret)
return ret;
if (!devm_add(dev, wrap_clk_bulk_put, &arg, sizeof(arg)))
return -ENOMEM;
return 0;
(+ consider devm_add() returning the error code instead, cfr. above).
BTW, I'm still wondering if the varargs macro discussed on #armlinux would
help. I.e.
devm_add(dev, wrap_clk_bulk_put, struct clk_bulk_devres, clks, num_clks)
would create and populate the temporary arg variable.
That would require defining an argument struct for the use in devm_clk_get(),
though.
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- [email protected]
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
On 22/01/2020 14:33, Geert Uytterhoeven wrote:
> On Wed, Jan 22, 2020 at 2:02 PM Marc Gonzalez wrote:
>
>> Introduce devm_add() to factorize devres_alloc/devres_add calls.
>>
>> Using that helper produces simpler code and smaller object size:
>>
>> 1 file changed, 27 insertions(+), 66 deletions(-)
>>
>> text data bss dec hex filename
>> - 1708 80 0 1788 6fc drivers/clk/clk-devres.o
>> + 1508 80 0 1588 634 drivers/clk/clk-devres.o
>>
>> Signed-off-by: Marc Gonzalez <[email protected]>
>
> Thanks for your patch!
>
>> --- a/drivers/base/devres.c
>> +++ b/drivers/base/devres.c
>> @@ -685,6 +685,20 @@ int devres_release_group(struct device *dev, void *id)
>> }
>> EXPORT_SYMBOL_GPL(devres_release_group);
>>
>> +void *devm_add(struct device *dev, dr_release_t func, void *arg, size_t size)
>
> Is there any advantage of using dr_release_t over "void (*action)(void *)",
> like devm_add_action() does? The latter lacks the "device *" parameter.
(I did forget to mention that v1 used devm_add_action.)
https://patchwork.kernel.org/patch/11262685/
A limitation of devm_add_action is that it stores the void *data argument "as is".
Users cannot pass the address of a struct on the stack. devm_add() addresses that
specific use-case, while being a minimal wrapper around devres_alloc + devres_add.
(devm_add_action adds an extra level of indirection.)
>> +{
>> + void *data = devres_alloc(func, size, GFP_KERNEL);
>> +
>> + if (data) {
>> + memcpy(data, arg, size);
>> + devres_add(dev, data);
>> + } else
>> + func(dev, arg);
>
> Both branchs should use { ...}
Ah yes, scripts/checkpatch.pl needs --strict to point this out.
>> +
>> + return data;
>
> Why return data or NULL, instead of 0 or -Efoo, like devm_add_action()?
My intent is to make devm_add a minimal wrapper (it even started out as
a macro). As such, I just transparently pass the result of devres_alloc.
Do you see an advantage in processing the result?
>> @@ -33,10 +25,7 @@ struct clk *devm_clk_get_optional(struct device *dev, const char *id)
>> {
>> struct clk *clk = devm_clk_get(dev, id);
>>
>> - if (clk == ERR_PTR(-ENOENT))
>> - return NULL;
>> -
>> - return clk;
>> + return clk == ERR_PTR(-ENOENT) ? NULL : clk;
>
> Unrelated change (which is less readable than the original, IMHO).
I'd like to hear the maintainers' opinion. I defer to their preference.
>> +
>> + if (!ret)
>> + if (!devm_add(dev, wrap_clk_bulk_put, &arg, sizeof(arg)))
>> + ret = -ENOMEM;
>
> Nested ifs are easier to read when the outer one uses curly braces:
>
> if (!ret) {
> if (!devm_add(dev, wrap_clk_bulk_put, &arg, sizeof(arg)))
> ret = -ENOMEM;
> }
>
> Or merge the condition with &&.
>
>>
>> return ret;
>
> But in this case, I would write it as:
>
> if (ret)
> return ret;
>
> if (!devm_add(dev, wrap_clk_bulk_put, &arg, sizeof(arg)))
> return -ENOMEM;
>
> return 0;
I like the simplicity of this code.
> (+ consider devm_add() returning the error code instead, cfr. above).
Some functions return an int, some a pointer, some might store the
result through a pointer.
> BTW, I'm still wondering if the varargs macro discussed on #armlinux would
> help. I.e.
>
> devm_add(dev, wrap_clk_bulk_put, struct clk_bulk_devres, clks, num_clks)
>
> would create and populate the temporary arg variable.
>
> That would require defining an argument struct for the use in devm_clk_get(),
> though.
There could be a helper for the "pass-a-struct" use-case, using a compound literal:
#define helper(dev, func, type, args...) devm_add(dev, func, &(type){args}, sizeof(type))
Regards.
Hi Marc,
On Thu, Jan 23, 2020 at 11:13 AM Marc Gonzalez <[email protected]> wrote:
> On 22/01/2020 14:33, Geert Uytterhoeven wrote:
> > On Wed, Jan 22, 2020 at 2:02 PM Marc Gonzalez wrote:
> >> Introduce devm_add() to factorize devres_alloc/devres_add calls.
> >>
> >> Using that helper produces simpler code and smaller object size:
> >>
> >> 1 file changed, 27 insertions(+), 66 deletions(-)
> >>
> >> text data bss dec hex filename
> >> - 1708 80 0 1788 6fc drivers/clk/clk-devres.o
> >> + 1508 80 0 1588 634 drivers/clk/clk-devres.o
> >>
> >> Signed-off-by: Marc Gonzalez <[email protected]>
> >> --- a/drivers/base/devres.c
> >> +++ b/drivers/base/devres.c
> >> @@ -685,6 +685,20 @@ int devres_release_group(struct device *dev, void *id)
> >> }
> >> EXPORT_SYMBOL_GPL(devres_release_group);
> >>
> >> +void *devm_add(struct device *dev, dr_release_t func, void *arg, size_t size)
> >
> > Is there any advantage of using dr_release_t over "void (*action)(void *)",
> > like devm_add_action() does? The latter lacks the "device *" parameter.
>
> (I did forget to mention that v1 used devm_add_action.)
> https://patchwork.kernel.org/patch/11262685/
>
> A limitation of devm_add_action is that it stores the void *data argument "as is".
> Users cannot pass the address of a struct on the stack. devm_add() addresses that
> specific use-case, while being a minimal wrapper around devres_alloc + devres_add.
> (devm_add_action adds an extra level of indirection.)
I didn't mean the advantage of devm_add() over devm_add_action(),
but the advantage of dr_release_t, which has a device pointer.
> >> +{
> >> + void *data = devres_alloc(func, size, GFP_KERNEL);
> >> +
> >> + if (data) {
> >> + memcpy(data, arg, size);
> >> + devres_add(dev, data);
> >> + } else
> >> + func(dev, arg);
> >> +
> >> + return data;
> >
> > Why return data or NULL, instead of 0 or -Efoo, like devm_add_action()?
>
> My intent is to make devm_add a minimal wrapper (it even started out as
> a macro). As such, I just transparently pass the result of devres_alloc.
>
> Do you see an advantage in processing the result?
There are actually two questions to consider here:
1. Is there a use case for returning the data pointer?
I.e. will the caller ever use it?
2. Can there be another failure mode than out-of-memory?
Changing from NULL to ERR_PTR() later means that all callers
need to be updated.
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- [email protected]
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
On 23/01/2020 11:32, Geert Uytterhoeven wrote:
> On Thu, Jan 23, 2020 at 11:13 AM Marc Gonzalez wrote:
>
>> A limitation of devm_add_action is that it stores the void *data argument "as is".
>> Users cannot pass the address of a struct on the stack. devm_add() addresses that
>> specific use-case, while being a minimal wrapper around devres_alloc + devres_add.
>> (devm_add_action adds an extra level of indirection.)
>
> I didn't mean the advantage of devm_add() over devm_add_action(),
> but the advantage of dr_release_t, which has a device pointer.
I'm confused...
void *devres_alloc(dr_release_t release, size_t size, gfp_t gfp);
int devm_add_action(struct device *dev, void (*action)(void *), void *data);
devres_alloc() expects a dr_release_t argument; devm_add() is a thin wrapper
around devres_alloc(); ergo devm_add() expects that dr_release_t argument.
devm_add_action() is a "heavier" wrapper around devres_alloc() which defines
a "private" release function which calls a user-defined "action".
(i.e. the extra level of indirection I mentioned above.)
I don't understand the question about the advantage of dr_release_t.
>>>> + void *data = devres_alloc(func, size, GFP_KERNEL);
>>>> +
>>>> + if (data) {
>>>> + memcpy(data, arg, size);
>>>> + devres_add(dev, data);
>>>> + } else
>>>> + func(dev, arg);
>>>> +
>>>> + return data;
>>>
>>> Why return data or NULL, instead of 0 or -Efoo, like devm_add_action()?
>>
>> My intent is to make devm_add a minimal wrapper (it even started out as
>> a macro). As such, I just transparently pass the result of devres_alloc.
>>
>> Do you see an advantage in processing the result?
>
> There are actually two questions to consider here:
> 1. Is there a use case for returning the data pointer?
> I.e. will the caller ever use it?
> 2. Can there be another failure mode than out-of-memory?
> Changing from NULL to ERR_PTR() later means that all callers
> need to be updated.
I think I see your point. You're saying it's not good to kick the can down
the road, because callers won't know what to do with the pointer.
Actually, I'm in the same boat as these users. I looked at
devres_alloc -> devres_alloc_node -> alloc_dr -> kmalloc_node_track_caller -> __do_kmalloc
Basically, the result is NULL when something went wrong, but the actual
error condition is not propagated. It could be:
1) check_add_overflow() finds an overflow
2) size > KMALLOC_MAX_CACHE_SIZE
3) kmalloc_slab() or kasan_kmalloc() fail
4) different errors on the CONFIG_NUMA path
Basically, if lower-level functions don't propagate errors, it's not
easy for a wrapper to do something sensible... ENOMEM looks reasonable
for kmalloc-related failures.
Regards.
Hi Marc,
On Thu, Jan 23, 2020 at 1:18 PM Marc Gonzalez <[email protected]> wrote:
> On 23/01/2020 11:32, Geert Uytterhoeven wrote:
> > On Thu, Jan 23, 2020 at 11:13 AM Marc Gonzalez wrote:
> >> A limitation of devm_add_action is that it stores the void *data argument "as is".
> >> Users cannot pass the address of a struct on the stack. devm_add() addresses that
> >> specific use-case, while being a minimal wrapper around devres_alloc + devres_add.
> >> (devm_add_action adds an extra level of indirection.)
> >
> > I didn't mean the advantage of devm_add() over devm_add_action(),
> > but the advantage of dr_release_t, which has a device pointer.
>
> I'm confused...
>
> void *devres_alloc(dr_release_t release, size_t size, gfp_t gfp);
> int devm_add_action(struct device *dev, void (*action)(void *), void *data);
>
> devres_alloc() expects a dr_release_t argument; devm_add() is a thin wrapper
> around devres_alloc(); ergo devm_add() expects that dr_release_t argument.
OK.
> devm_add_action() is a "heavier" wrapper around devres_alloc() which defines
> a "private" release function which calls a user-defined "action".
> (i.e. the extra level of indirection I mentioned above.)
>
> I don't understand the question about the advantage of dr_release_t.
OK. So devm_add_action() is the odd man out there.
> >>>> + void *data = devres_alloc(func, size, GFP_KERNEL);
> >>>> +
> >>>> + if (data) {
> >>>> + memcpy(data, arg, size);
> >>>> + devres_add(dev, data);
> >>>> + } else
> >>>> + func(dev, arg);
> >>>> +
> >>>> + return data;
> >>>
> >>> Why return data or NULL, instead of 0 or -Efoo, like devm_add_action()?
> >>
> >> My intent is to make devm_add a minimal wrapper (it even started out as
> >> a macro). As such, I just transparently pass the result of devres_alloc.
> >>
> >> Do you see an advantage in processing the result?
> >
> > There are actually two questions to consider here:
> > 1. Is there a use case for returning the data pointer?
> > I.e. will the caller ever use it?
> > 2. Can there be another failure mode than out-of-memory?
> > Changing from NULL to ERR_PTR() later means that all callers
> > need to be updated.
>
> I think I see your point. You're saying it's not good to kick the can down
> the road, because callers won't know what to do with the pointer.
Exactly.
> Actually, I'm in the same boat as these users. I looked at
> devres_alloc -> devres_alloc_node -> alloc_dr -> kmalloc_node_track_caller -> __do_kmalloc
>
> Basically, the result is NULL when something went wrong, but the actual
> error condition is not propagated. It could be:
> 1) check_add_overflow() finds an overflow
> 2) size > KMALLOC_MAX_CACHE_SIZE
> 3) kmalloc_slab() or kasan_kmalloc() fail
> 4) different errors on the CONFIG_NUMA path
>
> Basically, if lower-level functions don't propagate errors, it's not
> easy for a wrapper to do something sensible... ENOMEM looks reasonable
> for kmalloc-related failures.
Indeed. If devm_add() would return an error code, callers could just check
for error, and propagate the error code, without a need for hardcoding -ENOMEM.
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- [email protected]
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds